search

sherlock-audit / 2024-02-radicalxchange-judging

3 stars 1 forks source link

zzykxx - Users can cancel their own highest bid of any round, including the current one #15

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 8 months ago

zzykxx

high

Users can cancel their own highest bid of any round, including the current one

Summary

The highest bid of a round can be canceled by the bidder.

Vulnerability Detail

The function cancelAllBidsAndWithdrawCollateral() allows to cancel the highest bid of any round, including the current one.

The whole system works on the assumption that the highest bid cannot be canceled. This can cause all sorts of issues including DOS or funds theft. As an example, if an attacker is the current higher bidder of an auction that can be closed he can:

  1. Cancel his own highest bid via cancelAllBidsAndWithdrawCollateral(), which will add the bidded amount to his availableCollateral and then withdraw it immediately.
  2. Close the auction via closeAuction(), which will transfer the NFT to him.
  3. Attacker won the auction, got the NFT and recovered his bid.

Impact

Theft of funds, winning an auction without bidding anything.

Code Snippet

Tool used

Manual Review

Recommendation

In cancelAllBidsAndWithdrawCollateral() don't allow a bid to be canceled if it's the highest one. Don't revert but skip it with a continue inside the for loop.

Duplicate of #14