code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L157-L159 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L144 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L283 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L325 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L297 https://github.com/code-423n4/2022-11-size/blob/706a77e585d0852eae6ba0dca73dc73eb37f8fb6/src/SizeSealed.sol#L280-L285

Vulnerability details

Impact

The number of bids is limited to 1000. This is done to protect against a DoS attack where the array grows unbounded and makes finalization (almost) impossible. However a bidder can fill the auction with decreasing clearing prices in order to get their sized bid at the lowest possible price, possibly down to the reserve price (depending on the already existing bids).

Proof of Concept

The bid()function implements a protection against unbounded array as follows:

function bid(...) {
    ...
    if (bidIndex >= 1000) {
    revert InvalidState();
    }
}

A bidder can create a first sizeable bid at a high price and then create 999 other bids (or 999 - the number of already submitted bids) with low volume small amounts ( although they need to be higher than the minimumBidQuote) This would force the clearing price to be the lowest value before the bidIndex reaches 1000 or the totalBaseAmount has been filled. The clearing price is calculated here based on the clearingQuote and clearingBase input parameters which are required to be equal to the last accepted previousQuotePerBase after processing the complete bid list. This is however mitigated by the fact that the seller can chose not finalize the auction when they recognize this attack, but a seller might not be aware what's happening.

Tools Used

Manual review

Recommended Mitigation Steps

In stead of limiting the bids array to an arbitrary number, the finalization could be split in multiple steps choosing ranges of the array to process and only when the complete array has been processed actually finalize the auction and execute the payout. Another scenario could be to let the seller reveal and then allow the bidders to process their bids during finalization and only take into account the bids finalized by the bidders. This would however open up attacks where bidders try to game the system and not honor their bid in order to do so. A possible mitigation to this would be to only allow this approach when the number of bids exceeds a certain number.

c4-judge commented 2 years ago

0xean marked the issue as duplicate

c4-judge commented 1 year ago

0xean marked the issue as satisfactory