code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

A scheming bidder can manipulate the auction with knowledge of the bids placed by other users during reveal() phase by front running the finalize() transaction. This can result in seller of auction to earn much lesser than what he could and should have in a fair auction. I consider this high severity due to how likely it is to happen, causing seller to "lose" assets and at no additional cost to the malicious user since bids placed can be refunded.

Proof of Concept

Bids placed by other users in an auction is unknown during bidding phase. In a fair auction, if a user wants to win the bid, he should place a bid at a price that is the maximum he is willing to pay since he has no knowledge of what others will bid.

However, in this auction implementation, any user can cancelBid() after reveal() but before finalize(). This means that a bidder can place multiple bids over a wide range of prices to get the best price for himself. Depending on the bids placed by other bidders, the malicious bidder can cancel() the higher amount bids, hence forcing the clearingQuote price to be lower, benefitting himself as well as other bidders, at the expense of the seller. I use a simple example to illustrate this.

For example, a malicious bidder wants to purchase 1 token and wants to get it as the best possible price so he makes many different bids.

Auction is selling 5 tokens.

Say the current bids placed by other users are [50, 40, 30, 20, 10] per token.

Malicious bidder places bids of [85, 75, 65, 55, 45, 35, 25, 15, 5] per token.

In a normal situation, a bidder can only offer a bid amount of the maximum in which he is willing to part with. If this amount is >= 20, he would win the bid with the 4 other bidders who placed the bids of [50, 40, 30, 20] with clearingQuote = 20. However, we note that if the bidder is aware of bids placed by other users, he can optimally place a bid at price = 11 and still win the bid with a clearingQuote = 11, hence paying a cheaper price along with other bidders. Though this is not possible as a bidder cannot reasonably predict bids made by others.

However, because the malicious bidder can spread his bids out, and cancel all his bids except the one with price = 15 during reveal() phase, he can greatly decrease the price in which he has to pay to win the bid.

Tools Used

Manual Review

Recommended Mitigation Steps

This vulnerability is only possible because of the separate transactions made in reveal() and finalize(). I recommend to only allow seller to call both of these functions atomically.

trust1995 commented 2 years ago

"However, in this auction implementation, any user can cancelBid() after reveal() but before finalize()." I believe this to be incorrect , look at the code in cancelBid():

if (block.timestamp >= a.timings.endTimestamp) {
    if (a.data.lowestQuote != type(uint128).max || block.timestamp <= a.timings.endTimestamp + 24 hours) {
        revert InvalidState();
    }
}

0xean commented 2 years ago

   // Only allow bid cancellations while not finalized or in the reveal period

also believe this to be invalid.

c4-judge commented 2 years ago

0xean marked the issue as unsatisfactory: Invalid

code-423n4 / 2022-11-size-findings