AuctionDemo#claimAuction did not set auctionInfoData.status to false

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

AuctionDemo#claimAuction did not set auctionInfoData.status to false, the auction is still valid after the claim and the user can call cancelBid to get a 2x refund.

Proof of Concept

The claimAuction function finds the user with the highest bid and buys the NFT, and the other users make a refund.

    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}("");
                //@audit status has not been reset
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

Since auctionInfoData[_tokenid][i].status is not set to false, the user within the same block.timestamp can call cancelBid after claimAuction to get a 2x refund.

cancelBid verifies whether the Auction is out of date and auctionInfoData.status is true, if claimAuction and cancelBid are within the same block.timestamp, block.timestamp == minter.getAuctionEndTime(_tokenid), the time verification will pass, and since auctionInfoData.status is always true, the claimAuction and cancelBid functions will both be called successfully, and the user will get 2x For a refund.

    function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);
        auctionInfoData[_tokenid][index].status = false;
        (bool success, ) = payable(auctionInfoData[_tokenid][index].bidder).call{value: auctionInfoData[_tokenid][index].bid}("");
        emit CancelBid(msg.sender, _tokenid, index, success, auctionInfoData[_tokenid][index].bid);
    }

There are 3 ways a malicious user can carry out an attack:

1. bidder participates in the auction using a contract account(bidder is a contract account), after claimAuction is called, calling cancelBid in bidder's fallback, this is not a reentrant attack, it just makes cancelBid execute in the same block after the claimAuction execution.
2. claimAuction allows the winner to call, the winner can decide the time of call when block.timestamp <= minter.getAuctionEndTime(_tokenid), winner call claimAuction and then cancelBid to double the refund on one or more of his unwinning auctions.
3. Listen for on-chain transactions when the administrator invokes the claimAuction function to initiate cancelBid, which is then executed in the same block after claimAuction.

Tools Used

vscode manual

Recommended Mitigation Steps

Set status to false after claimAuction

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

a2rocket (sponsor) disputed

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as satisfactory