NextGen - crunch's results

Lines of code

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

Vulnerability details

Impact

User can win a token without paying the bid.

Proof of Concept

When a collection token is minted for auction, user place a bid through function participateToAuction(uint256 _tokenid) to participate. If a user's bid is the highest bid, the user can call function claimAuction(uint256 _tokenid) to claim the token.

    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}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

Function claimAuction allows user to claim at minter.getAuctionEndTime(_tokenid).

        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

However, user can also call function cancelBid(uint256 _tokenid, uint256 index) to cancel a bid at minter.getAuctionEndTime(_tokenid).

        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

It's possible that an auction winner claim the token at minter.getAuctionEndTime(_tokenid) and then cancel the bid at the same time, the winner gets the token without paying the bid.

Tools Used

Manual Review

Recommended Mitigation Steps

It's recommended that should only allow user to claim token after minter.getAuctionEndTime(_tokenid).

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

User can mint a burnToMint token and then sell the burned token.

Proof of Concept

If collection A token is able to be burned to mint a collection B token, user can call function burnToMint(uint256 mintIndex, uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o, address burner) to burn the collection A token and get a collection B token in return.

    function burnToMint(uint256 mintIndex, uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o, address burner) external {
        require(msg.sender == minterContract, "Caller is not the Minter Contract");
        require(_isApprovedOrOwner(burner, _tokenId), "ERC721: caller is not token owner or approved");
        collectionAdditionalData[_mintCollectionID].collectionCirculationSupply = collectionAdditionalData[_mintCollectionID].collectionCirculationSupply + 1;
        if (collectionAdditionalData[_mintCollectionID].collectionTotalSupply >= collectionAdditionalData[_mintCollectionID].collectionCirculationSupply) {
            _mintProcessing(mintIndex, ownerOf(_tokenId), tokenData[_tokenId], _mintCollectionID, _saltfun_o);
            // burn token
            _burn(_tokenId);
            burnAmount[_burnCollectionID] = burnAmount[_burnCollectionID] + 1;
        }
    }

New collection B token will be first minted to the user and then the collection A token will be burned.

            _mintProcessing(mintIndex, ownerOf(_tokenId), tokenData[_tokenId], _mintCollectionID, _saltfun_o);
            // burn token
            _burn(_tokenId);

If in a secondary market, a buyer placed a buy order for a collection A token and deposited the funds, for seller to make a transaction to sell the collection token and receive the funds. A malicious user who owns the collection A token can call function burnToMint from a contract, the a collection B token will be minted to the contract and callback function onERC721Received on the contract will be called. The malicious user can then sell the collection A token to the buyer and get the funds, then the token is burned and the buyer gets nothing.

function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        // Sell the collection A token to the secondary market
        ...
    }

Tools Used

Manual Review

Recommended Mitigation Steps

It's recommended that collection A token should be burned before collection B token is minted.

Assessed type

ERC721

Lines of code

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

Vulnerability details

Impact

User may not be able to get their funds back after auction ends.

Proof of Concept

When an auction ends, the winner or admin calls function claimAuction(uint256 _tokenid) to claim the token. The token will be transferred to the winner.

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

Here, function safeTransferFrom(address from, address to, uint256 tokenId) and _checkOnERC721Received(address from, address to, uint256 tokenId, bytes memory data) will be called eventually. If the winner is a contract, function _checkOnERC721Received will call the callback function onERC721Received in the receiving contract and it will fail unless the receiving contract properly implements the ERC721TokenReceiver interface.

        if (to.isContract()) {
            try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) {
                return retval == IERC721Receiver.onERC721Received.selector;
            } catch (bytes memory reason) {
                if (reason.length == 0) {
                    revert("ERC721: transfer to non ERC721Receiver implementer");
                } else {
                    /// @solidity memory-safe-assembly
                    assembly {
                        revert(add(32, reason), mload(reason))
                    }
                }
            }
        }

It's possible that the winner contract does not implement the ERC721TokenReceiver interface properly, then the claiming will fail. However, user can only get refund through the function claimAuction.

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

A failed claiming results in users being unable to get their funds back.

Tools Used

Manual Review

Recommended Mitigation Steps

It's recommended that user should be able to get refund without function claimAuction.

Assessed type

DoS