sherlock-audit / 2024-02-radicalxchange-judging

3 stars 1 forks source link

0rpse - _cancelAllBids does not check if bid is the highest #60

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 8 months ago

0rpse

high

_cancelAllBids does not check if bid is the highest

Summary

EnglishPeriodicAuctionInternal::_cancelAllBids lacks checks to determine if bids are one of the highest bids in auction rounds. This will allow highest bidder to withdraw collateral while keeping the highest bidding spot.

Vulnerability Detail

When a bidder places a bid it has to be the highest bid, also when a bidder cancels a bid it has to not be the highest bid, these invariants protect the English Auction from bidders gaming the auction and breaking internal accounting. As the highest bid check is non existent in _cancelAllBids and as _cancelAllBids does not reset highestBids mapping accordingly, a user can place the highest bid, cancel it and withdraw collateral while keeping the highest bid.

Impact

An adversary can get Stewardship without paying collateral to the previous Steward.

Code Snippet

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/459dfbfa73f74ed3422e894f6ff5fe2bbed146dd/pco-art/contracts/auction/EnglishPeriodicAuctionInternal.sol#L416-L434 https://github.com/sherlock-audit/2024-02-radicalxchange/blob/459dfbfa73f74ed3422e894f6ff5fe2bbed146dd/pco-art/contracts/auction/facets/EnglishPeriodicAuctionFacet.sol#L187-L190

Tool used

Manual Review

Recommendation

Add necessary check to see if cancelled bid is the highest bid similar to _cancelBid.

Duplicate of #14