NextGen - cccz's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/71d055b623b0d027886f1799739b7f785b5bc7cd/smart-contracts/AuctionDemo.sol#L124-L130

Vulnerability details

Impact

In auctionDemo.cancelBid, the highest bid can be canceled, which allows the user to revoke the highest bid at the end of the auction and then use a smaller price to bid for the NFT.

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

Consider the following scenario where Alice and Bob are bidding on an NFT. Alice bids 1 ETH. Bob bids 100 ETH. At the last moment, Bob cancels his bid, then bids 1.00001 ETH and gets the NFT.

Proof of Concept

https://github.com/code-423n4/2023-10-nextgen/blob/71d055b623b0d027886f1799739b7f785b5bc7cd/smart-contracts/AuctionDemo.sol#L124-L143

Tools Used

None

Recommended Mitigation Steps

It is recommended to disallow canceling the highest bid in cancelBid and cancelAllBids.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

In auctionDemo, participateToAuction(), cancelBid() and cancelAllBids() should be allowed to be called before the auction ends, while claimAuction() should be allowed to be called after the auction ends.

But when block.timestamp == minter.getAuctionEndTime(_tokenid), all four functions can be called, which will cause many serious problems, the most serious of which can cause users to steal ETH in the contract.

When block.timestamp == minter.getAuctionEndTime(_tokenid), claimAuction() is called to send the ETH of failed auction or NFT to the user. However, since claimAuction() does not update auctionInfoData.status to false, the user can continue to call cancelBid() to withdraw the ETH from the contract.

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

Consider the following scenario, Alice bids 100 ETH for NFT1, Bob bids 101 ETH for NFT2, and the contract balance is 201 ETH. When block.timestamp == minter.getAuctionEndTime(NFT1), claimAuction() is called to send NFT to Alice and 100 ETH to owner. At this time, the contract balance is 101 ETH. At this point Alice can call cancelBid(), which will cause the contract to send 100 ETH to Alice.

Proof of Concept

https://github.com/code-423n4/2023-10-nextgen/blob/71d055b623b0d027886f1799739b7f785b5bc7cd/smart-contracts/AuctionDemo.sol#L104-L142 https://github.com/code-423n4/2023-10-nextgen/blob/71d055b623b0d027886f1799739b7f785b5bc7cd/smart-contracts/AuctionDemo.sol#L57-L61

Tools Used

None

Recommended Mitigation Steps

Change to

    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);
        auctionClaim[_tokenid] = true;

Assessed type

Context