NextGen - tnquanghuy0512's results

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

Lines of code

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

Vulnerability details

Impact

In AuctionDemo.claimAuction(), the contract will tranfer the NFT to the highest bidder of that NFT by using ERC721.safeTransferFrom() method. The issue here is, if the highest bidder is a contract and doesn't have onERC721Received() function or not return the magic number correctly, the function AuctionDemo.claimAuction() will get DOS forever. Moreover, all the bidders can't withdraw their fund using cancelBid() or cancelAllBids() because the auction end time was passed. And because this contract doesn't have an emergency function, the NFT and all the bid will get stuck in it.

Proof of Concept

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
        ...
        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 {}
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

-   IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
+   IERC721(gencore).transferFrom(ownerOfToken, highestBidder, _tokenid);

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Notice that when block.timestamp == minter.getAuctionEndTime(_tokenid), users can call to both 2 contract participateToAuction() and claimAuction(). Because of that, this scenario can happens:

• User A is currently the highest bidder of the NFT X, so when block.timestamp == minter.getAuctionEndTime(_tokenid), he call to claimAuction() to rightfully claim his NFT X
• User B doesn't know the NFT X is claimed, he call to participateToAuction() to bid the highest price for NFT X when block.timestamp == minter.getAuctionEndTime(_tokenid)
• User B realize that the NFT X is claimed, so he call to cancelBid() to withdraw his bid, but he can't because the time is already surpass minter.getAuctionEndTime(_tokenid)
• The fund got stuck forever in the contract

Proof of Concept

Notice that user can call to both 2 function participateToAuction() and claimAuction() when block.timestamp == minter.getAuctionEndTime(_tokenid)

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

Tools Used

Manual Review

Recommended Mitigation Steps

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

Assessed type

Other