SIZE contest - chaduke's results

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L391

Vulnerability details

Impact

Detailed description of the impact of this finding. A malicious user can perform the following steps in ONE transaction to DOS attack the auction system by front-running the first bid transaction:

1. submit a bid to auction with bidIndex = 0;
2. cancel the bid with bidIndex = 0;
3. submit a bid to auction with bidIndex = 1
4. cancel the bid with bidIndex = 1;
5. Repeat steps 3 and 4 for bidIndex = 2 to 999 By now, all the bidIndex from 0-999 have been used and they can never be reused. The bidder has not bid any. No future bidders can bid anymore due to the current bidInde = 1000. The malicous user has successfully prevent anyone from bidding.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L415

Tools Used

Remix

Recommended Mitigation Steps

We need to take the following mitigation steps:

1. Record the timestamp of a bidding transaction;
2. The cancelBid() transaction cannot be performed in the same block as the bid transaction for the same bidIndex.
3. change the logic of the bid transaction so that unused bidIndex can be reused by another bidder.

G1: https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L144-L146 Since quoteAmount < a.params.minimumBidQuote already covers the case quoteAmount == 0, so we can drop condition 1 to save gas

if (quoteAmount == type(uint128).max || (quoteAmount < a.params.minimumBidQuote)) {
            revert InvalidBidAmount();
}

G2: https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L223 The privateKey == 0 check should be performed at the beginning of the reveal function to save gas.

G3: https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L241 use shifting to save gas

uint256[] memory seenBidMap = new uint256[]((bidIndices.length >> 8 )+1);

G4. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L270-L277 InvalidSorting should be checked with a separate function and then be called at the beginning of finalize().

G5. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L217 Dupliate detection in argument bidIndices should be accomplished by another inner function, which should be called at the beginning of finalize().

G6. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L313-L315 This check is not necessary since in the for loop (L289-291), we ensure that we will never exceed the data.totalBaseAmount.

G7. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L28-L43 for the modifier atState, we can cache a.times.startTimestamp and a.times.endTimestamp to save gas as they are accessed multiple times

G8. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L376 Cache b.quoteAmount as it is accessed twice.

G9. https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L425 Cache a.timing.endTimestamp as it is accessed twice