Race condition at auction end time: User can both participate and claim

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/a6f2397b68ef2865374c1bf7629349f25e71a44d/smart-contracts/AuctionDemo.sol#L105
https://github.com/code-423n4/2023-10-nextgen/blob/a6f2397b68ef2865374c1bf7629349f25e71a44d/smart-contracts/AuctionDemo.sol#L58

Vulnerability details

Impact

In the AuctionDemo contract, users can bid in an auction until its deadline and then claim the NFT. However, the current code allows users to call both participateToAuction(...) and claimAuction(...) at the auction end time.

This could lead to situations where:

1. A winner may try to claim the winning NFT, but another user might outbid them.
2. If a user bids at the last moment, the existing winner might claim the NFT before the new bid is processed.

Proof of Concept

The code snippets below show how auctionEndTime is checked in both functions. As you can see, both functions can be called when timestamp == auctionEndTime.

function participateToAuction(uint256 _tokenid) public payable {
    // @audit Allow when timestamp <= auctionEndTime
    require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
    ...
}

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    // @audit Allow when timestamp >= auctionEndTime
    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
    ...
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider only allowing 1 action (either participate or claim) at auction end time.

Assessed type

Timing

[c4-pre-sort]

141345 marked the issue as duplicate of #1935

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1926

[c4-judge]

alex-ppg marked the issue as partial-50