NextGen - zhaojie's results

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

Lines of code

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

Vulnerability details

Impact

There is a reentrant attack in AuctionDemo#claimAuction, users can auction for tokens at zero cost.

Proof of Concept

The claimAuction function sends tokens to the winning bidder via IERC721(gencore).safetransferfrom.

bidder can be a contract account, bidder implements onERC721Received, calls cancelBid in onERC721Received to get a refund, and bidder can get the token with zero payment.

    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) {
                //@audit reentrant ,calling `cancelBid` in the `onERC721Received` callback function
                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

vscode manual

Recommended Mitigation Steps

Add openzeppelin reentrancy guard.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

block.timestamp = collectionPhases.allowlistStartTime || block.timestamp = collectionPhases.publicEndTime

getPrice returns the wrong price

Proof of Concept

When salesOption==2 in the getPrice function, there is no case where the time is equal to the start/end time.

When block.timestamp is equal to allowlistStartTime or publicEndTime, the default price is returned instead of the price over time.

It may be correct to return the default time for allowlistStartTime, but it is wrong to return the default price for publicEndTime.

function getPrice(uint256 _collectionId) public view returns (uint256) {
    ....
    } else if (
        collectionPhases[_collectionId].salesOption == 2 &&
        //@audit missing =
        block.timestamp > collectionPhases[_collectionId].allowlistStartTime &&
        block.timestamp < collectionPhases[_collectionId].publicEndTime
    )

In NextGenMinterContract#mint the judgment of time is: >= <= So when block.timestamp = publicEndTime/allowlistStartTime, the mint parameter is set correctly.

function mint(
        uint256 _collectionID,
        uint256 _numberOfTokens,
        uint256 _maxAllowance,
        string memory _tokenData,
        address _mintTo,
        bytes32[] calldata merkleProof,
        address _delegator,
        uint256 _saltfun_o
    ) public payable {
        ....
        if (
            block.timestamp >= collectionPhases[col].allowlistStartTime &&
            block.timestamp <= collectionPhases[col].allowlistEndTime
        ) {
        ....
        } else if (
            block.timestamp >= collectionPhases[col].publicStartTime &&
            block.timestamp <= collectionPhases[col].publicEndTime
        ) {
        ....
        require(msg.value >= (getPrice(col) * _numberOfTokens), "Wrong ETH");
        for (uint256 i = 0; i < _numberOfTokens; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
            gencore.mint(
                mintIndex,
                mintingAddress,
                _mintTo,
                tokData,
                _saltfun_o,
                col,
                phase
            );
        }
        ....

Tools Used

vscode manual

Recommended Mitigation Steps

    function getPrice(uint256 _collectionId) public view returns (uint256) {
        ....
     } else if (
            collectionPhases[_collectionId].salesOption == 2 &&
            block.timestamp >= collectionPhases[_collectionId].allowlistStartTime &&
            block.timestamp <= collectionPhases[_collectionId].publicEndTime
        )

Assessed type

Other