Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L116-L126 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L112 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L157-L177 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L461-L466 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L415 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/NextGenCore.sol#L307-L311

Vulnerability details

Impact

A malicious admin has too much power on all the collection states and can makes multiples actions (intentionally or not) and steal tokens to the user, prevent user to mint by reducing the declared time of a mint period, etc

Proof of Concept

Before the listing of potentials abuse of the admins, i would like to link this issues of a previous C4 contest to demonstrate the severity of all this potentials issues to be Medium. I make this list in the goal to highlight that even if the admin role is trusted and use a multisig wallet, a malicious or not manipulation can break some parts of the contracts and impact users (stealing tokens, change periods break a lot of functions and computations, etc) or artist (stealing pay) who use the protocol and represent weaknesses for the protocol. https://github.com/code-423n4/2022-02-badger-citadel-findings/issues/50#issuecomment-1066631798

Like we can see in the modifiers of contracts NextGenCore, MinterContract, etc. The admin can access almost all of the functions:

    require(... || adminsContract.retrieveGlobalAdmin(msg.sender) == true, "Not allowed");

There are multiples cases of potentials rug:

1) Admin prevent an auctionned token to be distributed to the winner of the auction by revoking the approval. In MinterContract::mintAndAuction(), the Admin send the token to a chosen contract, and in claimAuction() auction the NFT is transferred by a safeTransferFrom() where the approval can be revoked by the admin. If the approval is revoked, the user can't retrieve the token and users can't withdraw funds used for bidding.

2) In MinterContract Admin & CollectionAdmin can change the differents costs and phases of a collection at anytime, it can lead to sandwich attack on a user transaction and user minting in a higher amount, preventint a user to mint, etc

    function setCollectionCosts(uint256 _collectionID, uint256 _collectionMintCost, uint256 _collectionEndMintCost, uint256 _rate, uint256 _timePeriod, uint8 _salesOption, address _delAddress) public CollectionAdminRequired(_collectionID, this.setCollectionCosts.selector) {
        require(gencore.retrievewereDataAdded(_collectionID) == true, "Add data");
        collectionPhases[_collectionID].collectionMintCost = _collectionMintCost;
        collectionPhases[_collectionID].collectionEndMintCost = _collectionEndMintCost;
        collectionPhases[_collectionID].rate = _rate;
        collectionPhases[_collectionID].timePeriod = _timePeriod;
        collectionPhases[_collectionID].salesOption = _salesOption;
        collectionPhases[_collectionID].delAddress = _delAddress;
        setMintingCosts[_collectionID] = true;
    }

    // function to add a collection's start/end times and merkleroot

    function setCollectionPhases(uint256 _collectionID, uint _allowlistStartTime, uint _allowlistEndTime, uint _publicStartTime, uint _publicEndTime, bytes32 _merkleRoot) public CollectionAdminRequired(_collectionID, this.setCollectionPhases.selector) {
        require(setMintingCosts[_collectionID] == true, "Set Minting Costs");
        collectionPhases[_collectionID].allowlistStartTime = _allowlistStartTime;
        collectionPhases[_collectionID].allowlistEndTime = _allowlistEndTime;
        collectionPhases[_collectionID].merkleRoot = _merkleRoot;
        collectionPhases[_collectionID].publicStartTime = _publicStartTime;
        collectionPhases[_collectionID].publicEndTime = _publicEndTime;
    }

3) An admin can use MinterContract::emergencyWithdraw() to simply drain all the pending artists payment. 4) Admin can use the setPrimaryAndSecondarySplits(), proposePrimaryAddressesAndPercentages() and acceptAddressesAndPercentages() function in the MinterContract and he has power to choose the percentage splits of an artist without his validation. The docs says:

"The setPrimarySplits(..) function is used to set the primary percentage splits for a specific collection for both the artist and the NextGen Team."

Admin can simply set all the percentage to the team or admin and not pay the artist. Also if the artist is not added in "FunctionAdmin", artist can't call the payArtist() function and only admin can choose if he pay the artist or not. It can be abusive if the team don't want to pay the artist, but it's the artist creation.

5) Admin can change the collectionPhases[_collectionID].publicEndTime and call the NextGenCore::setFinalSupply() to reduce the maximum supply at any time. A user who knows he'll be able to mint because the supply is high may be prevented from minting by the admin, intentionally or not.

Tools Used

Manual Review

Recommended Mitigation Steps

1) When a token is auctionned, send the token to the AuctionDemo contract and in this way there is no problem of abusive approval. 2) Prevent CollectionAdmin or admin to change sensitive parameters during configured/sensitive phases. 3) Avoid admin using MinterContract::emergencyWithdraw() if artist is not paid. 4) Change the ways the admin can access the function, let only the artist set the percentages splits (and potentially hardcoded the percentage of the team) and admin validate the percentages. Also, let the artist trigger the function enabling him to receive his funds whenever he wishes. 5) Like in the "2)", prevent the admin from modifying the collection phases when they have already been defined.

Assessed type

Rug-Pull

code-423n4 / 2023-10-nextgen-findings

Multiples ways for admins to rug users/artist #1572