NextGen - circlelooper's results

Lines of code

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

Vulnerability details

Impact

Highest bidder can cancel the bid right after claiming the token.

Proof of Concept

A token minted for auction, it can be claimed by the highest bidder right after the auction ends.

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

The bid can also be cancelled at the end of the auction.

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

It can be seen that if block.timestamp is minter.getAuctionEndTime(_tokenid), the token can be claimed and cancelled in the same transaction.

    function getAuctionEndTime(uint256 _tokenId) external view returns (uint) {
        return mintToAuctionData[_tokenId];
    }

This results in the highest bidder receives the token but not pays a price.

Tools Used

Manual Review

Recommended Mitigation Steps

Please consider to change the requirement and not allow the token to be claimed at auction end time.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

Owner of burnToMint token can frontrun secondary sales.

Proof of Concept

The collection token can be traded on secondary market, since they are standard ERC721 tokens.

In a Burn-to-Mint sale model, a collection token can be burned to minted a different collection token, this is done through burnToMint method.

The minted token will be transferred to the receiver through ERC721's _safeMint method, which will call _checkOnERC721Received method if the receiver is a contract.

Then the collection token used for minting will be burned.

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

The problem is the _checkOnERC721Received method in the receiver contract, a bad actor may sell the to-be-burned collection token after receiving the new collection token, the buyer of the to-be-burned collection token will get the token and lose it immediately, because the token will be burned then.

Tools Used

Manual Review

Recommended Mitigation Steps

Please consider to burn the to-be-burned collection token, then mint the new collection token.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

Auction refunding can be DOSed by malicious winner.

Proof of Concept

Auction winner is supposed to claim token after an auction, or the admins can claim for him, the other bidders will be refunded at the claiming.

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        ...
        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 {}
        }
    }

This method will loop through all the bidders, if the bidder is the winner, the token will be transferred to the winner through safeTransferFrom method, if the winner is a contract, onERC721Received method will be called in the receiver contract.

    function _checkOnERC721Received(
        address from,
        address to,
        uint256 tokenId,
        bytes memory data
    ) private returns (bool) {
        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))
                    }
                }
            }
        } else {
            return true;
        }
    }

If the bidder is not the winner, the ether will be refunded to the bidder.

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

If the winner is being malicious and revert on purpose in the onERC721Received method, then the whole claiming will fail and the other bidders won't be refunded and their ether will be locked.

Please note it's not necessarily mean that the winner will lose the token as he can make the receiver contract to accept the token at any time he wants to.

Tools Used

Manual Review

Recommended Mitigation Steps

Please consider to separate refunding from claiming, so the bidders can be refunded even if the claiming is failed.

Assessed type

Access Control