sherlock-audit / 2023-02-fair-funding-judging

1 stars 0 forks source link

seyni - `refund_highest_bidder` can be frontrun by a call to `settle` #81

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

seyni

medium

refund_highest_bidder can be frontrun by a call to settle

Summary

Anyone can frontrun the owner calling refund_highest_bidder by calling settle which leads for the owner to potentially not be able to call the function for any highest_bidder of auctions.

Vulnerability Detail

Anyone can call settle as soon as the current auction ends. As a consequence, any call to refund_highest_bidder by the owner can be frontrun by a call to settle which will start the next auction and set _epoch_in_progress() to True and set highest_bidder to empty(address).

Impact

The owner will potentially not be able to call refund_highest_bidder for any highest_bidder of auctions.

Code Snippet

AuctionHouse.vy#L315-L325

def refund_highest_bidder():
    assert msg.sender == self.owner, "unauthorized"
    assert self._epoch_in_progress() == False, "epoch not over"

    refund_amount: uint256 = self.highest_bid
    refund_receiver: address = self.highest_bidder

    self.highest_bid = 0
    self.highest_bidder = empty(address)

    ERC20(WETH).transfer(refund_receiver, refund_amount)

Tool used

Manual Review

Recommendation

I recommend making settle only callable by the owner or a permissionned role.

Unstoppable-DeFi commented 1 year ago

refund_highest_bidder is only here in case settle would revert on some underlying issue on Alchemix, a winning bidder would never be refunded under normal operations since this would be unfair to all other bidders.

hrishibhat commented 1 year ago

Closing based on Sponsor comment