addRandomizer() and setTokenHash() functions don't check if _collectionID exist

[c4-submissions]

Lines of code

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

Vulnerability details

In 'NextGenCore.sol' contract 'addRandomizer()' function adds randomizer to a chosen _collectioId. The problem is that the function is missing a check if the chosen _collectionId does exist. Same is with 'setTokenHash' in NextGenCore.sol, RandomizerNXT, RandomizerRNG, RandomizerVRF contracts.

    function addRandomizer(uint256 _collectionID, address _randomizerContract) public FunctionAdminRequired(this.addRandomizer.selector) {
        require(IRandomizer(_randomizerContract).isRandomizerContract() == true, "Contract is not Randomizer");
        collectionAdditionalData[_collectionID].randomizerContract = _randomizerContract;
        collectionAdditionalData[_collectionID].randomizer = IRandomizer(_randomizerContract);
    }

    function setTokenHash(uint256 _collectionID, uint256 _mintIndex, bytes32 _hash) external {
        require(msg.sender == collectionAdditionalData[_collectionID].randomizerContract);
        require(tokenToHash[_mintIndex] == 0x0000000000000000000000000000000000000000000000000000000000000000);
        tokenToHash[_mintIndex] = _hash;
    }

For example in 'updateCollectionInfo()' function there is a check if the _collectionId is created.

        require((isCollectionCreated[_collectionID] == true) && (collectionFreeze[_collectionID] == false), "Not allowed");

Impact

addRandomizer() and setTokenHash() are not restricted over working with invalid _collectionId value meaning that they can add invalid data for non-existing _collectionId in the protocol.

Proof of Concept

    function addRandomizer(uint256 _collectionID, address _randomizerContract) public FunctionAdminRequired(this.addRandomizer.selector) {
        require(IRandomizer(_randomizerContract).isRandomizerContract() == true, "Contract is not Randomizer");
        collectionAdditionalData[_collectionID].randomizerContract = _randomizerContract;
        collectionAdditionalData[_collectionID].randomizer = IRandomizer(_randomizerContract);
    }

    function setTokenHash(uint256 _collectionID, uint256 _mintIndex, bytes32 _hash) external {
        require(msg.sender == collectionAdditionalData[_collectionID].randomizerContract);
        require(tokenToHash[_mintIndex] == 0x0000000000000000000000000000000000000000000000000000000000000000);
        tokenToHash[_mintIndex] = _hash;
    }

Also setTokenHash() is used in multiple contracts like NextGenCore.sol, RandomizerNXT, RandomizerRNG, RandomizerVRF. By not checking if the collectionId is valid this can lead to recording invalid data to the state of the contract

Recommended Mitigation Steps

Add the following check in setTokenHash() and addRandomizer() functions:

        require(isCollectionCreated[_collectionID] == true);

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as duplicate of #478

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as primary issue

[alex-ppg]

The Warden specifies that the assignment of a randomizer and the setTokenHash functions do not validate that the collection for which they are being executed exists.

This finding relies on reckless misuse of an administrative function (assignment of a randomizer) and as such, I consider it to be invalid. We can safely assume that the administrator will ensure the randomizer they are configuring is being done so for a valid collection.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid