NextGen - nuthan2x's results

Lines of code

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

Vulnerability details

Impact

Severity : Medium

Proof of Concept

• In MinterContract.mint function, when public sale is happening, there's a check shown below to check for max allowed to mint per address. But it can bypassed to mint more per address.

require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");

• This is do to the Check effect Interaction pattern is not followed on Gencore.mint function

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

    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;
            }
+           _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
        }
    }

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Severity : High (Ether loss)

Proof of Concept

When Auction ends (block.timestamp == AuctionEndTime),

• If the higher bidder calls at exact end time, when sending back ether to the oher bidders, they can reenter the cancelBid function to steal the ether.
• The impact will be huge if cancelAllBids reentered.
• The loss happens to the other collectionId auction bids.

Here's he exact steps to execute this attack:

Attack way 1:

1. When block.timestamp == AuctionEndTime, by attacker being a highest bidder, call claimAuction
2. The highest bidder receives the nft and can reenter the cancelAllBids function to claimhis bid.
3. The attacker could frontrun the prev high bidder who is now claiming, he will frontrun with highest bid and then gets the token and cancels the bid to get his bidded etehr back.

Attack way 2:

1. When block.timestamp == AuctionEndTime, by attacker being a highest bidder, call claimAuction
2. When ether is sent to the rest of the low bid bidders, they will reenter the cancelAllBids to also claim extra ether.

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

Follow any of these mitigation:

• Follow the beloww changes or add reentrancy locks mechanism by openZeppelin.

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);
        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);
                (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}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {
            }
        }
    }





## Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Severity : High Likelihood : low

Proof of Concept

• lastMintDate on salesOption 3 depends on circulating supply which doesnt count in burnAmount. So circulating supply can be inflated so high due to the if block on genCore.mint will not trigger when total supply is exceeded, but circulating supply is updated without minting.

• So, after inflating, if salesOption is changed from 1 or 2 to 3. The lastMint date is hugely inflated now, we cant mint new tokens. Ofcourse the collection owner should change the following to let this happen:

  • Chnage salesOption & totalsupply have to be increased

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

• Make last mint calulation to be depended on previous last mint data.

• Modify MinterContract.mint's if block

    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));
+       lastMintDate[col] += timeperiod
    }

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

Severity : High (ex: a salesOption that increases price over time, will incur a heavy loss if token id minted at exact publicEndTime)

Proof of Concept

• In MinterContract.mint function, when minting, MinterContract.getPrice can be abused for different pricing for same collection. Due to following improper timestamp validation.

1. User mints exactly at collection's allowlistStartTime or publicEndTime.
2. Now the price of that tokens will be different from rest of collection, because the else if block validated for current timestamp to be < or > to allowlistStartTime & publicEndTime instead of <= and >=.
3. so if this is not fixed, the sers can mint exactly at allowlistStartTime or publicEndTime and get away with exact collectionMintCost instead of price increase or decrease due to the time difference, number of mints, rate and other collection costs parameters

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

function getPrice(uint256 _collectionId) public view returns (uint256) {
        uint tDiff;
        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;
            }
-       } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp > collectionPhases[_collectionId].allowlistStartTime && block.timestamp < collectionPhases[_collectionId].publicEndTime){
+       } else if (collectionPhases[_collectionId].salesOption == 2 && block.timestamp >= collectionPhases[_collectionId].allowlistStartTime && block.timestamp <= collectionPhases[_collectionId].publicEndTime){
            tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod;
            uint256 price;
            uint256 decreaserate;
            if (collectionPhases[_collectionId].rate == 0) {
                price = collectionPhases[_collectionId].collectionMintCost / (tDiff + 1);
                decreaserate = ((price - (collectionPhases[_collectionId].collectionMintCost / (tDiff + 2))) / collectionPhases[_collectionId].timePeriod) * ((block.timestamp - (tDiff * collectionPhases[_collectionId].timePeriod) - collectionPhases[_collectionId].allowlistStartTime));
            } else {
                if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
                    price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
                } else {
                    price = collectionPhases[_collectionId].collectionEndMintCost;
                }
            }
            if (price - decreaserate > collectionPhases[_collectionId].collectionEndMintCost) {
                return price - decreaserate; 
            } else {
                return collectionPhases[_collectionId].collectionEndMintCost;
            }
        } else {
            // fixed price
            return collectionPhases[_collectionId].collectionMintCost;
        }
    }

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Severity : High (loss of rest bidder funds) likelihood : medium (the highest bidder should be a contract)

Proof of Concept

When Auction ends, the highest bidder can claim the nft, but being a contractas highest bidder can resfuse to implement onERC721Received fallback, can cause DOS. So the owner of token will not get his ether. And there's no way to refund the bidders if this attack is executed, since you cannot cancel bids after auction ends.

Here's he exact steps to execute this attack:

1. call minter.mintAndAuction() which airdrops the token to recipient and created auction status to true.
2. Now all the bidding takes place and end time of auction is passed.
3. The highest bidder can refuse to accept the ERC721 safeFransfer, so claimAuction will always revert. And the owner of that tokenId it airdropped to can also refust to accept the ether, by beibng a contract (low likelihood).
4. Bidders can't even get refunds by calling cancelBid functions because its past the endTime, and now the ether is stuck.

Tools Used

Manual review & foundry testing

Recommended Mitigation Steps

In AuctionDemo.participateToAuction, use transfer instead of safeTrnafer. Th attacker can afford to lose if doesn't implement the ERC721 receiving standard.

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);
+            IERC721(gencore).transferFrom(ownerOfToken, highestBidder, _tokenid);
            (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}("");
            emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
        } else {
        }
    }
}

Assessed type

ERC721