search

sherlock-audit / 2024-02-radicalxchange-judging

3 stars 1 forks source link

0xShitgem - Not implemented restriction inside `EnglishPeriodAuctionInternal::_cancelAllBids` leads to possibility of stealing funds by attacker #38

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

0xShitgem

high

Not implemented restriction inside EnglishPeriodAuctionInternal::_cancelAllBids leads to possibility of stealing funds by attacker

Summary

Not implemented similar restriction like in _cancelBid for _cancelAllBids can lead to stealing funds.

Vulnerability Detail

_cancelAllBids like name suggest is cancelling every bid user made in current round. Functions is working as expected (it cancel bids), however it doesn't check - like _cancelBid - if msg.sender is highest bidder in current round.

Malicious user - Bob - can use that to his advantage:

  1. Firstly Bob needs to win his Steward License (SL).
  2. Later, when another round is starting for SL - Bob from his second account (let's call Bob2) is putting as bidAmount absurdly high amount of ETH (in PoC I used 10 ETH). Basically the higher bid is, then Bob has higher chance of winning. Additionally he never lose money, because in worst case scenario, when Bob lost - he can withdrawCollateral
  3. After putting bid from Bob2, immidietaly he calls function cancelAllBidsAndWithdrawCollateral - thanks to that his ETH balance is the same as before putting bid (minus gas fees).
  4. When auction closed, Bob receives for his availableCollateral balance the amount Bob2 put as a bidAmount.
  5. Bob2 withdraws his collateral

When balance of auction smart contract is less then collateral of Bob2 - he won't be able to withdraw. However I think this scenario is highly improbable, because majority of people won't be likely to pay gas fees everytime after auction is closed. Especially in times of high gas fees.

Proof Of Code

Impact

User can steal other people's funds from EnglishPeriodicAuctionFacet smart contract.

Code Snippet

https://github.com/sherlock-audit/2024-02-radicalxchange/blob/main/pco-art/contracts/auction/EnglishPeriodicAuctionInternal.sol#L416-L434

Tool used

Manual Review

Recommendation

Recommend adding the same check - bidder != l.highestBids[tokenId][round].bidder as in _cancelBid

Duplicate of #14