Users can lose their funds when participating to a claimed Auction

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Users will lose their funds for ever if they participate to claimed auctions.

Proof of Concept

The admin of the protocole is able to open an auction on a newly minted token by simply calling the NextGenMinterContract.mintAndAuction(), this function will mint a new token and send it to the specified address owner. Then, the function will open an auction for that specific token.

After that, users can call the auctionDemo.participateToAuction() function to participate to a specific token auction by sending ETH and specifying the tokenId.

    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);
    }

In addition, when the auctionEndTime arrives, the winner (user with the highest bid) will be able to claim his auction by calling the following function:

    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 {}
        }
    }

However, as you can see on both lines 105 and 58, when the block.timestamp == minter.getAuctionEndTime(_tokenid) the winner will be able to claim his auction and the other users will still be able to participate in auctions.
unfortunately, if the winner claim the auction first, all the users that will participate to that claimed auction will lose their funds for ever because:

1. the only way to retrieve participante funds is by calling the claimAuction() function
2. and because that function will keep reverting each time it is called as auctionClaim[_tokenid] will be != false once the auction is claimed.
3. and because the cancelBid() function will also revert due to require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

Here is a step by step Proof of Concept to reproduce this vulnerability:

/**
Before running this PoC please enable the `allowBlocksWithSameTimestamp` for hardhat networks to be able to run both claimAuction() and participateToAuction() in the same block.timestamp. Also don't forget to deploy the auctionDemo contract.
*/
it("#mintAndAuctionNFTCol3", async function () {
      // Step 1: call mintAndAuction() to mint and create an Auction
      const timestampBefore = (await ethers.provider.getBlock('latest')).timestamp + 100
      await contracts.hhMinter.mintAndAuction(
        signers.addr1.address, // _recipient
        'abcd', // _tokenData
        0, // _saltfun_o
        3, // _collectionID
        timestampBefore // _auctionEndTime
        //{ value: await contracts.hhMinter.getPrice(3) }
      )

      const tokenId = 30000000001;

      // Step 2: approve AuctionDemo to transfer the minted token to 
      console.log("[+] Approving transfer for AuctionDemo from owner !")
      await contracts.hhCore.connect(signers.addr1).approve(contracts.hhAuctionDemo.getAddress(), 30000000001);

      // Step 3: participate to an auction to be able to claim it later
      console.log("[+] Participate to an auction to be able to claim it later !")
      await contracts.hhAuctionDemo.participateToAuction(30000000001, { value: await contracts.hhMinter.getPrice(3) })

      // Saving the auction Eth balance after the first participation
      const auctionDemoBalanceAfterFirstParticipation = await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())

      /*console.log((await ethers.provider.getBlock('latest')).timestamp)
      const auctionDemoBalanceBefore = await contracts.hhCore.balanceOf(
        contracts.hhAuctionDemo.getAddress(), // _address
      )*/

      // Step 4: Set the timestamp of the block to the date where auction End
      await time.setNextBlockTimestamp(timestampBefore);
      console.log("[+] End of participation timestamp reached : " + (await ethers.provider.getBlock('latest')).timestamp)

      // Step 5: claiming the auction by the winner
      await contracts.hhAuctionDemo.claimAuction(30000000001)

      // the balance should not be decreased it stay the same as the NFT was sold to the only participante
      console.log("[+] Auction contract balance after claim : " + (await contracts.hhCore.balanceOf(
        contracts.hhAuctionDemo.getAddress(), // _address
      )))

      // Save AuctionDemo contract balance after the claim to auctionDemoOldBalance
      const auctionDemoBalanceAfterClaim = await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())

      // Reset the timestamp to the auction ending timestamp (this is to simulate that auction participation and auction claim are performed at the same block one after the other)
      await time.setNextBlockTimestamp(timestampBefore);

      // Step 6: Participate to an already claimed auction of the token 30000000001
      await contracts.hhAuctionDemo.connect(signers.addr2).participateToAuction(30000000001, { value: (await contracts.hhMinter.getPrice(3) * BigInt(2)) })

      // Save AuctionDemo contract balance after the claim to auctionDemoOldBalance
      const auctionDemoBalanceAfterSecondParticipation = await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())

      console.log("auctionDemoBalanceAfterFirstParticipation : " + auctionDemoBalanceAfterFirstParticipation);
      console.log("auctionDemoBalanceAfterClaim : " + auctionDemoBalanceAfterClaim);
      console.log("auctionDemoBalanceAfterSecondParticipation : " + auctionDemoBalanceAfterSecondParticipation);

      expect(auctionDemoBalanceAfterSecondParticipation).equal((await contracts.hhMinter.getPrice(3) * BigInt(2)));

    })

Tools Used

Manual

Recommended Mitigation Steps

There is two ways to solve this issue:

1. by changing the timestamp condition for participating to block.timestamp < minter.getAuctionEndTime(_tokenid)
2. Or adding auctionClaim[_tokenid] == false check to the participate require() call

Assessed type

Other

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