Lack of input validation

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L157-L167
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L170-L177
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L319
https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L320-L321

Vulnerability details

Impact

Initialization, modification functions used by admin don't have any internal validation. These may lead to protocol breaks, such as: wrong state, blocking collections and so on. Such errors may be as special, or as "fat finger" error.

Proof of Concept

Let's discuss some code parts:

        collectionPhases[_collectionID].collectionMintCost = _collectionMintCost;
        collectionPhases[_collectionID].collectionEndMintCost = _collectionEndMintCost;
        collectionPhases[_collectionID].rate = _rate;
        collectionPhases[_collectionID].timePeriod = _timePeriod;
        collectionPhases[_collectionID].salesOption = _salesOption;

Here possible problems:

• collectionEndMintCost less than collectionMintCost, this leads to errors during price calculation;
• timePeriod can be 0, this leads to error during price calculation;
• salesOption can be out of supported values (1,2,3).

        collectionPhases[_collectionID].allowlistStartTime = _allowlistStartTime;
        collectionPhases[_collectionID].allowlistEndTime = _allowlistEndTime;
        collectionPhases[_collectionID].merkleRoot = _merkleRoot;
        collectionPhases[_collectionID].publicStartTime = _publicStartTime;
        collectionPhases[_collectionID].publicEndTime = _publicEndTime;

• Possible to create time ranges where end time > start time;
• Possible to create crossed time ranges;
• Possible to create public sales before allowlist sales.

        burnOrSwapAddress[externalCol] = _burnOrSwapAddress;
        burnOrSwapIds[externalCol][0] = _tokmin;
        burnOrSwapIds[externalCol][1] = _tokmax;

• _tokmin can be less than _tokmax;
• _burnOrSwapAddress can be 0, this leads to stop burnToMint process, because transfer to address 0 will be reverted.

To sum up:
Better add validations for these input parameters, because it helps to prevent occasional typos on main-net.

Tools Used

Manual review

Recommended Mitigation Steps

Add validations for incoming parameters in all modification functions.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-sponsor]

a2rocket (sponsor) disputed

[a2rocket]

the contract size is fully, there is no place to do not necessary checks, if something is wrong it can be updated.

[alex-ppg]

Combination of several issues like #1980, #1831, and #2033. The statements on each finding apply to this one, rendering it invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid