NextGen - peanuts's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L113

Vulnerability details

Impact

Protocol will get the highest bid amount instead of the NFT owner during the auction process.

Proof of Concept

In claimAuction(), the owner of the ERC721 token is set as ownerOfToken.

       address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);

The winner will get the ERC721 from the ownerOfToken through the safeTransferFrom function. The contract will then pay owner(), which is the contract owner, instead of ownerOfToken, which is the NFT owner.

       IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
        (bool success, ) = payable(owner()).call{value: highestBid}("");

owner() is used in the Ownable contract, which is also inherited by NextGenCore.sol.

    /**
     * @dev Returns the address of the current owner.
     */
    function owner() public view virtual returns (address) {
        return _owner;
    }

This means that the auction money will go to the protocol owner instead of the NFT owner.

If the protocol intends for the money to go to the protocol instead of the NFT owner, the money should not go to the owner of the AuctionDemo contract but rather the MinterContract itself. That way, the amount earned from the auction can be split amongst the artist and the team, like how it is done through mint(). After the auction, send the money to the minter contract and increase the value of collectionTotalAmount in the miinter contract. The admin can then call payArtist() to split the amount accordingly.

collectionTotalAmount[col] = collectionTotalAmount[col] + msg.value;

Tools Used

Manual Review

Recommended Mitigation Steps

Change owner() to ownerOfToken.

       IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
-        (bool success, ) = payable(owner()).call{value: highestBid}("");
+        (bool success, ) = payable(ownerOfToken).call{value: highestBid}("");

or otherwise send the money to the minter contract.

Assessed type

Token-Transfer

Lines of code

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

Vulnerability details

Impact

The latest highest bidder will lose his funds if he participated in the auction after claimAuction() is called in the same timestamp.

Proof of Concept

Let's understand how participation and winning works first.

When participating in the auction, the function participateToAuction() checks whether the bid sent is higher than the highest bid and checks the auction end time. (Take note of the <= sign of the auction end time)

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

If the current highest bid is 1 ETH, a participant can only bid an amount higher than 1 ETH.

So the participants will keep bidding until the time is > the auction end time. If the end time is 1pm, the participants cannot bid after 1pm, but can still bid at 1pm on the dot.

Next, let's look at how the process works once the auction is over. claimAuction() is called. The function checks whether the time is >= the auction end time. auctionClaim[_tokenid] == false means that this function can only be called once.

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

If the claimAuction function is 1pm, at 1pm and onwards, the claimAuction() function can be called. Once claimAuction() is called, there will be no more refunds to participants. cancelBid() cannot be called as well.

The Issue

• User A is currently the highest bidder at 10 ETH. The auction end time is 1pm and the current time is 1pm.
• User A calls claimAuction() to get his ERC721 NFT.
• All other participants get their ETH back and User A gets his NFT.
• Right after claimAuction() is called, user B calls participateToAuction() and puts 11 ETH. (time is still 1pm)
• User B is able to participate because the block.timestamp is equal to the auction end time. User B is now the highest bidder.
• However, claimAuction() is called already and cannot be called again.
• 1 second passes, User B cannot call cancelBid() anymore.
• User B has 11 ETH stuck in the contract.

This issue can happen quite often, especially if the auction is a speed auction (only 5 seconds to participate). Also, since NextGen is able to host multiple collection, there can be multiple auction happening, and this issue will definitely surface. Since the highest bidder can also control the claimAuction() function, he can call the function on the end time and 'frontrun' the next highest bidder, which will lead to the bidder's ETH getting stuck in the contract.

Tools Used

VSCode

Recommended Mitigation Steps

A simple fix is to make sure that claimAuction() can only be called > auction end time, and not >= auction end time. This prevents any errors and only the highest bidder can win the NFT.

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

Assessed type

DoS

Overview of the Protocol

• The NextGen Protocol helps artists to mint an NFT collection. All NFT created by NextGen will be part of the NextGen Collection.
• The artist can create up to 10 billion NFTs per collection
• The protocol supports different types of minting fee, such as fixed fee, linear/exponential descending fee and fixed percentage ascending fee.
• The protocol takes a percentage cut from every NFT sale.
• There are some unique functions that NextGen has. Firstly, a user can burn an NFT created through NextGen to mint an NFT in a NextGen Collection through the burnToMint() function.
• A user can also burn an external NFT (subject to protocol's approval) to mint an NFT in a NextGen Collection through burnOrSwapExternalToMint().
• NFTs can also be minted and auctioned by the admin using the mintAndAuction() function and the AuctionDemo contract.

Walkthrough of the Protocol

• As an artist, the first step is for the artist's team to get in touch with the protocol team.
• The protocol will call createCollection() in NextGenCore.sol, and provide the necessary details like collection name, description, etc.
• The protocol will then register a collection admin from the artist team.
• The collection admin will then call setCollectionData() and provide more details on the collection like total supply, max collection purchase limit.
• After setting the collection data, the collection admin can set the collection cost and collection phases in the MinterContract, which includes the mint price, sales option, mint duration.
• At the same time, the protocol admin will set the primary and secondary splits and the percentages. This indicates how much percentage the team will earn for every minted NFT.
• Once timing and costs has been set, users can start to interact with the protocol and call mint().
• The money earned from the NFT sales will be stored in the MinterContract. To retrieve the money, payArtist() is called, and the payment is distributed to the artist team and the protocol team respectively.
• Once the mint duration is up, the admin can call setFinalSupply to set the final supply of the collection

To create another collection, the whole process will restart.

Codebase Quality Analysis and Review

MinterContract.sol (Deep Dive)

mint()

• :white_check_mark: phase 1, only allow list, have delegator (delegation management contract OOS)
• :white_check_mark: phase 1, only allow list, no delegator (delegation management contract OOS)
• :white_check_mark: phase 2, public access
• viewTokensIndexMin(col) is a fixed number
• If salesOption == 3, only can mint 1 token.
• Allowlist needs a merkleProof
• tDiff = number of periods that has passed

getPrice()

• :white_check_mark: salesOption == 3, with rate set (price will increase every mint, rate set in percentage)
• :white_check_mark: salesOption == 3, without rate set(price will stay the same every mint, rate set in percentage)

        if (collectionPhases[_collectionId].salesOption == 3) {
            // increase minting price by mintcost / collectionPhases[_collectionId].rate every mint (1mint/period)
            // to get the price rate needs to be set
            if (collectionPhases[_collectionId].rate > 0) {
                return collectionPhases[_collectionId].collectionMintCost + ((collectionPhases[_collectionId].collectionMintCost / collectionPhases[_collectionId].rate) * gencore.viewCirSupply(_collectionId));
            } else {
                return collectionPhases[_collectionId].collectionMintCost;

• :white_check_mark:salesOption != 3 or 2 (price will stay the same every mint)

        } else {
            // fixed price
            return collectionPhases[_collectionId].collectionMintCost;

• :white_check_mark:salesOption == 2, without rate set (exponential decrease until resting price, rate set in wei)
• checked with timePeriod = 1 hour, mint price = 0.5e18 ETH
• If time passed = 1 min, tdiff = 0, decreaserate = 0.25e18 / 1 hours * 1min = 0.00416, price = 0.5e18 - 0.00416e18 = 0.4958e18
• If time passed = 30 min, tdiff = 0, decreaserate = 0.25e18 / 1 hours * 30min = 0.125e18, price = 0.5e18 - 0.125e18 = 0.375e18
• If time passed = 50 min, tdiff = 0, decreaserate = 0.25e18 / 1 hours * 50min = 0.208e18, price = 0.5e18 - 0.208e18 = 0.292e18
• If time passed = 1 hours, tdiff = 1, decreaserate = 0, price = 0.25e18
• If time passed = 1.5 hours, tdiff = 1, decreaserate = (0.25e18 - (0.5e18/3)) / 3600 * 1800 = 4.16e16, price = 0.25e18 - 4.16e16 = 0.208e18
• If time passed = 2 hours, tdiff = 2, decreaserate = 0, price = 0.5 / 3 = 0.16666e18

Seems like the exponential descending calculation is correct

• :white_check_mark:salesOption == 2, rate set (linear decrease until resting price, rate set in wei)
• checked with timePeriod = 1 hour, mint price = 0.5e18 eth, min price = 0.1e18 eth, rate is 1e17
• If time passed = 30 mins, tdiff = 0, 4 > 0, price = 0.5e18 eth - (0 * 1e17) = 0.5e18 eth
• If time passed = 1 hours, tdiff = 1, 4 > 1, price = 0.5e18 eth - (1e17) = 0.4e18 eth
• If time passed = 3.5 hours, tdiff = 3, 4 > 3, price = 0.5e18 eth - 3e17 = 0.2e18 eth
• If time passed = 5 hours, tdiff = 5, 4 < 5, else clause, price = collectionMintCost = 0.1e18 eth

Seems like the linear descending calculation is correct

Preliminary Questions

MinterContract

1. Is the admin of ArtistOrAdminRequired and FunctionAdminRequired the same?
2. Why does proposePrimaryAddressesAndPercentages and proposeSecondaryAddressesAndPercentages have 3 addresses specifically?
3. What is the point of proposeSecondaryAddressesAndPercentages if it's not used in payArtist?
4. Why is the mint function so long? What is the function trying to accomplish? Line 196
5. Whats the difference between salesOption == 3 and salesOption == 2 and salesOption == (other number)? Line 532 onwards
6. What is phase 1 and phase 2? Line 203, 222
7. What is airdropTokens?
8. What does burnToMint mean?

Preliminary Answers

Minter Contract

1. Yes, they are the same.
2. Probably nothing, just design.
3. Not sure.
4. The mint function distinguishes between allowlist timings and public timings, and whether the token minted has a delegator.

• Looking at the code, from Line 202 to 220 in the minter contract, the mint function focuses on the allowlist users only.
• The first if statement checks that the caller sets a delegator (Line 205), and the else statement means that the caller is minting for himself (Line 215).
• Inside both if and else statements, _maxAllowance is checked, calling the gencore.retrieveTokensMintedALPerAddress, making sure that the allowlisted address do not mint past the max allowance.
• collectionTokenMintIndex is checked. (checked, it tallies up)
• Mint is called in gencore contract. Circulation supply is increased and checked against total supply. (checked, it tallies up)
• _mintProcessing is called which invokes ERC721
• If salesOption == 3, only 1 mint per time period (checked, tallies up when timePeriod = 1 days (86400 seconds))
• (timeOfLastMint will be 1 day before allowliststart, the first mint of a collection is allowed)
• (lastMintDate will then update to the the time of allowliststart, the second mint can only take place 1 day after)
• salesOption == 3 means that the price will be linear, one NFT will be more expensive than the next
• salesOption == 2, means price decreasing exponentially or linearly, depending on whether rate is set (linearly = rate is set)
• salesOption == other, means fixed NFT price

5. The difference salesOption indicates the pricing of NFT

• salesOption == (other number) means that the NFT will always be at a fixed price
• salesOption == 3 means only 1 mint allowed every time. If rate is set, the mint price will go up linearly. Rate here indicates the percentage. If rate is not set, the nft will be at fixed price
• salesOption == 2 means the NFT price will decrease until a minimum price. If rate is set, the price will decrease exponentially. If rate is not set, the price will decrease linearly. Rate is stated in wei.

6. Phase 1 is allowlist access and phase 2 is public access. Phase 1 must be earlier than phase 2.
7. AirdropTokens can only be used by the admin, it gives an array of recipient one NFT each for free (recipients don't have to pay any money)
8. burnToMint means burning an older token to mint a new token. From the code, mintIndex is the index to be minted, and _tokenId is the token to be burned. Users can burn an NFT in their nextGen collection and mint another NFT in the nextGen collection, provided they are given permission by the admin.

Centralization Risks

Architecture Review

Pros of using NextGen

• Convenient for the artist. Most of the minting process is covered by the protocol team
• If a user have a NextGen collection NFT, he can burn it for another NextGen NFT (with admin's approval). This can introduce a tier system for NFTs.
• Clear payment procedure and royalty splits.
• Lots of room to create unique, generative NFTs

Cons of using NextGen

• User can only pay in native token
• The admin has a extremely huge control over the process, including payments. There must be a lot of trust in NextGen Protocol when creating a collection on the platform.

Approach, Comments and Learnings

• Thoroughly auditied the whole scope, except for the delegation, merkle proof and strings part.
• Interesting to see how a exponential descending price is coded.
• Protocol also doesn't use external imports like OpenZeppelin but instead copied all the contracts. Pretty interesting style, simple and convenient, but may lack upgrade capabilities.
• The NextGen protocol can house many collections without any Id collisions.

Time spent:

40 hours