carrot - Starting timestamp can be bypassed by calling `settle`

[github-actions[bot]]

carrot

high

Starting timestamp can be bypassed by calling settle

Summary

The starting timestamp set or still unset by the owner through the start_auction function can be bypassed by calling settle, which sends the first token to the fallback and then starts the auction for subsequent tokenIds.

Vulnerability Detail

The function start_auction is meant to be used to start the auction process, after which the bids start getting accepted. However, this entire system can be bypassed by calling the settle function. This leads to the first tokenId being minted to the fallback address, and the next tokenId auction being started immediately.

This can be exploited in two scenarios,

1. The function start_auction hasn't been called yet
2. The function start_auction has been called, and the timestamp passed is a timestamp in the future

In both these cases, the auctions can be made to start immediately. Thus the two issues are clubbed together.

The function settle only checks for the timestamp using the statement https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/AuctionHouse.vy#L185 which is defined as https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/AuctionHouse.vy#L247-L252 This check passes if the current timestamp is after the end of the epoch, but also if the current timestamp is before the start of the auction, which is the main issue here.

Inside the settle function, it sets the start and end timestamps properly, which allows bids to be made for subsequent tokenIds. https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/AuctionHouse.vy#L200-L206

So even if the starting timestamp is unset or set in the future, the checks in settle pass, and the function then proceeds to write the start and end timestamps to process bids correctly.

Impact

Bids can be started immediately. This goes against the design of the protocol.

Code Snippet

The issue can be recreated with the following POC

def test_ATTACK_settle_before_start(owner, house, nft):
    token_id = house.current_epoch_token_id()
    assert house.highest_bidder() == pytest.ZERO_ADDRESS
    house.settle()
    house.bid(house.current_epoch_token_id(), house.RESERVE_PRICE())
    assert house.current_epoch_token_id() == token_id + 1
    assert nft.ownerOf(token_id) == owner

This shows a case where start_action is never called, yet the bids start. The same can be done if start_auction is called with a timestamp in the future

Tool used

Boa Manual Review

Recommendation

Change the check in settle to check for the end timestamp ONLY

assert block.timestamp > self.epoch_end

[hrishibhat]

While the issue is valid, there are no funds at risk with starting the auction early. Considering this issue a valid medium.

[Unstoppable-DeFi]

https://github.com/Unstoppable-DeFi/fair-funding/pull/7

[HickupHH3]

Fix looks good:

• auction_started boolean flag is used to prevent settle() from being called before an auction begins
• auctions can now only be started once, and can never be restarted.