THE RANDOM HASH GENERATED VIA THE `RandomizerNXT.calculateTokenHash` FUNCTION IS NOT TRULY RANDOM

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/RandomizerNXT.sol#L55-L59 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L36 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L41

Vulnerability details

Impact

The RandomizerNXT.calculateTokenHash function is used to calculate the random hash and returns it to the gencore contract. The random hash is calculated as shown below:

    bytes32 hash = keccak256(abi.encodePacked(_mintIndex, blockhash(block.number - 1), randoms.randomNumber(), randoms.randomWord()));

Here the randoms.randomNumber() and randoms.randomWord()) (here randoms is an instance of the XRandoms contract) functions are called to obtain the respective randomness to the calculation of the random hash.

The XRandoms.randomNumber function and XRandoms.randomWord function uses the following computations to calculate the randomNum respectively, as shown below:

    uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 1000;

    uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;        

But the issue is that the solidity documentation says block.number and block.timestamp should not be relied upon for randomness as shown below:

Do not rely on block.timestamp or blockhash as a source of randomness, unless you know what you are doing.

Further block.prevrandao is also manipulatable since the randomness from the past is known and predictable, as explained in the following article: https://soliditydeveloper.com/prevrandao

As a result the random hash value calculated in the RandomizerNXT.calculateTokenHash is not truly random since its randomness is predictable and can be manipulated by the validators. Hence the NextGenCore.tokenToHash value for a specific TokenId can be manipulated as a result. Since the tokenToHash[tokenId] is used in the NextGenCore.retrieveGenerativeScript function to create the Generative Script of a token and this generative script is used to calculate the tokenURI of a token when the onchainMetadata[tokenIdsToCollectionIds[tokenId]] == true, it can be concluded that the tokenURI of a ERC721 token is manipulatable when the RandomizerNXT is used as the randomizer contract of a specific collection (Since the returned random hash via the calculateTokenHash function is not truly random).

Proof of Concept

    function calculateTokenHash(uint256 _collectionID, uint256 _mintIndex, uint256 _saltfun_o) public {
        require(msg.sender == gencore);
        bytes32 hash = keccak256(abi.encodePacked(_mintIndex, blockhash(block.number - 1), randoms.randomNumber(), randoms.randomWord()));
        gencoreContract.setTokenHash(_collectionID, _mintIndex, hash);
    }

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/RandomizerNXT.sol#L55-L59

       uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 1000;

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L36

        uint256 randomNum = uint(keccak256(abi.encodePacked(block.prevrandao, blockhash(block.number - 1), block.timestamp))) % 100;

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/XRandoms.sol#L41

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to only use the RandomizerRNG and RandomizerVRF contracts for the random hash generation during the ERC721 token miniting process.

Or else recommended to pick a future prevrandao value when using the block.prevrandao as explained in the https://soliditydeveloper.com/prevrandao article. This will ensure block.prevrandao can be used in a more secure way to generate the randomness.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as duplicate of #163

[c4-judge]

alex-ppg marked the issue as duplicate of #1901

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid