The ETH that user participates in the auction may be locked forever in some special scenarios

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/hardhat/smart-contracts/AuctionDemo.sol#L57-L61

Vulnerability details

Impact

The auctionDemo contract's participateToAuction function does not provide good condition verification.

When the auction ends, users can still participate. If a user calls the participateToAuction function at the auction's end timestamp, the ETH he provided for the bid may be permanently locked in the contract because the NFT may have already been claimed by the winner.

Proof of Concept

Let's assume that the timestamp of the end of the auction is exactly equal to the timestamp of a block.

Alice and Bob both want to win the NFT, and they broadcast their transactions at the same time within the last 12 seconds:

1. Assume that Alice has bid 1 ETH and is currently the winner of the entire auction. She broadcasts a transaction calling the claimAuction function to successfully claim the NFT.

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
    auctionClaim[_tokenid] = true;
    uint256 highestBid = returnHighestBid(_tokenid);
    address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
    address highestBidder = returnHighestBidder(_tokenid);
    for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
        if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
            IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
            (bool success, ) = payable(owner()).call{value: highestBid}("");
            emit ClaimAuction(owner(), _tokenid, success, highestBid);
        } else if (auctionInfoData[_tokenid][i].status == true) {
            (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
            emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
        } else {}
    }
}

2. Bob's transaction follows closely, and he broadcasts a transaction offering 2 ETH to call the participateToAuction function to participate in the bidding.

function participateToAuction(uint256 _tokenid) public payable {
    require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
    auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
    auctionInfoData[_tokenid].push(newBid);
}

3. The above transactions are completed simultaneously in the last block. At this point, Bob's 2 ETH will be locked in the contract forever!

4. Because the NFT has been taken by Alice, so Bob's call to the claimAuction function will not succeed. And the timestamp of the next block will be greater than the auction end timestamp, so Bob cannot call the cancelBid function to take his ETH back .

Tools Used

Foundry And VsCode

Recommended Mitigation Steps

optimize the participateToAuction function

function participateToAuction(uint256 _tokenid) public payable {
    require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
+   require(auctionClaim[_tokenid] == true, " not allow to participate anymore ");    
    auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
    auctionInfoData[_tokenid].push(newBid);
}

Assessed type

Invalid Validation

[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 satisfactory

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)