Adding a randomizer to a collection should be mandatory

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The minting process will be broken.

Proof of Concept

In the current setup, a randomizer contract can be associated with each collection, and it plays a crucial role in generating a random value for token uniqueness during the minting process.

Here's a breakdown of the process:

1. Initially, a collection doesn't have a randomizer. An admin can add a randomizer using the addRandomizer function. Importantly, this step is currently not mandatory to proceed with the collection setup and initiate the sale.

2. If a randomizer is not set for a collection, the collection will lack a randomizer contract, and the associated address will be address(0).

3. However, there's a potential issue in the _mintProcessing() function. This function in NextGenCore.sol attempts to call the calculateTokenHash() function in the randomizer contract linked to the collection ID. If no randomizer is set, this call will fail, leading to transaction reversal and breaking the entire minting process.

This issue needs attention to ensure a smooth minting process, even when a randomizer is not explicitly set for a collection. Consideration should be given to handle scenarios where a randomizer is not defined to prevent transaction failures.

Tools Used

VSCode

Recommended Mitigation Steps

Given the crucial role of the randomizer contract in the minting process, it's important to implement a solution that ensures a smooth workflow, even when a randomizer hasn't been explicitly set for a collection. Here are two potential approaches:

Set Default Randomizer: Consider setting a default randomizer contract for collections where a randomizer hasn't been explicitly defined. This default randomizer could serve as a fallback, preventing transaction failures and ensuring the continuity of the minting process.

Make Randomizer Addition Mandatory: Alternatively, you can make it mandatory to add a randomizer before progressing to the next step, specifically before executing the setCollectionCosts() function. This ensures that each collection has a valid randomizer contract associated with it before moving forward in the setup process:

Choosing between these approaches depends on the overall design goals and the desired user experience. Both options address the potential issue highlighted in the _mintProcessing() function:

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-pre-sort]

141345 marked the issue as duplicate of #478

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

The Warden specifies that the absence of a randomizer would break its sale process as no token hash would be generateable.

As can be observed in the codebase, the collection's configuration has a different ACL applied than the collection's randomizer configuration. This is presumably because the NextGen team wishes to support a subset of "validated" randomizers rather than arbitrary contracts.

As such, I consider this exhibit invalid. A valid QA would be to advise the configuration of a collection mandates that a randomizer has been set by the NextGen system, however, the Warden has failed to advise this in a clear manner and has over-inflated the severity of their submission.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Overinflated severity

[c4-judge]

alex-ppg marked the issue as primary issue