search

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

1 stars 0 forks source link

Malicious users may launch DOS attacks or ensure that they buy at the lowest price. #118

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Malicious users may stop all or most of other users from submitting bids(DOS).

Malicious users may ensure that they buy the base token at the lowest price.

Proof of Concept

If a user ( X ) wants to DOS an auction, he/she only needs to send up to 1000 invalid bids. As the bid() function limits 1000 bids per auction, all subsequent from other users bids will fail. Because all bids are invalid, the auction will ends with no successful bidder.

If a user ( X ) wants to buy some or all tokens of an auction, he/she only needs to send one valid bid with the lowest price (reserveQuotePerBase) and a batch up to 999 of invalid bids. Because there are only lowest price bids and invalid bids, X will buy with the lowest price successfully.

Invalid bids are easy to construct, such as using a price below the lowest price (reserveQuotePerBase), or using an invalid public key, or using an invalid commitment, etc.

X can retrieve all quote tokens of those invalid bids by cancelBid() or refund().

What X cost is just some gas fee, and it can be reduced by calling bid() in batches from a custom contract and using more zero parameters (pubkey, commitment, etc).

Both situations can not be avoided even if the auction adopted the whitelist mechanism.

Tools Used

VS Code

Recommended Mitigation Steps

There are 3 solutions to consider:

  1. Adopting the whitelist mechanism, and limit each user can only have one bid per auction.

In order to support users to cancel or change a bid(cancel or change quote price), we can replace the cancelbid() with modifyBid().

The key of this solution is that whitelists must be carefully selected.

  1. Remove the limit of 1000 bids per auction, and let the seller only submit valid bids in finalize() without all invalid or low-price bids.

This solution can completely avoid those two problems, but introduced the assumption that sellers are always rational.

That is, we assume that sellers will always choose the bids with higher price to maximize their income. Fortunately, this assumption should hold true most of the time.

  1. Remove the limit of 1000 bids per auction, and modify the finalize() to support finalize an auction by calling finalize() multiple times.

If there are too many bids in one auction, one finalize() call cannot handle all the bids because of the ethereum gas limit. So we have to support handling a part of the bids in one finalize().

This solution can completely avoid those two problems, but introduced the assumption that sellers are always rational. However this modification may be a bit complicated, need some extra storage may need to be used to record the intermediate state.

c4-judge commented 2 years ago

0xean marked the issue as duplicate

c4-judge commented 1 year ago

0xean marked the issue as satisfactory

c4-judge commented 1 year ago

0xean changed the severity to 2 (Med Risk)