Randomness for a tokenId can be lost if RNG contract runs out of ETH or VRF subscription runs out of LINK

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/RandomizerVRF.sol#L65
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/RandomizerRNG.sol#L48

Vulnerability details

Impact

The RandomizerVRF.sol and RandomizerRNG.sol contracts are used to generate randomness for each token on NextGen collections. In both these contracts, there are functions requestRandomWords() and fulfillRandomWords(). The former is called to request randomness for a token while the latter is called by the VRF or RNG service to serve the randomness, which then further calls the setTokenHash() function on the gencore contract to store the hash generated. The problem arises when the VRF or RNG service runs out of LINK in the subscription plan or ETH in the contract respectively. Due to this, the call to serve randomness to fulfillRandomWords() and setTokenHash() fails, which leads to the randomness being lost for that tokenId. The hash for the tokenId cannot be set through some other way since setTokenHash() is only callable from the Randomizer contracts, which are further only used when minting tokens in the NextGenCore.sol contract. Since the user cannot mint a token with the same tokenId again, the hash is permanently empty for that given tokenId.

Proof of Concept

Here is the process:

1. requestRandomWords() is called to request randomness for the tokenId being minted

File: RandomizerVRF.sol
52:     function requestRandomWords(uint256 tokenid) public {
53:         require(msg.sender == gencore);
54:         uint256 requestId = COORDINATOR.requestRandomWords(
55:             keyHash,
56:             s_subscriptionId,
57:             requestConfirmations,
58:             callbackGasLimit,  
59:             numWords
60:         );
61:         tokenToRequest[tokenid] = requestId;
62:         requestToToken[requestId] = tokenid;
63:     }

2. VRF or RNG service calls fulfillRandomWords() to serve randomness for the tokenId being minted. But since there is not enough LINK in the VRF subscription or ETH in the RNG contract, the call fails and the randomness for that tokenId being minted is lost. This leads to an empty token hash for the user who minted that tokenId and thus an incorrect NFT display (since script uses tokenHash).

File: RandomizerVRF.sol
65:     function fulfillRandomWords(uint256 _requestId, uint256[] memory _randomWords) internal override {
66:         gencoreContract.setTokenHash(tokenIdToCollection[requestToToken[_requestId]], requestToToken[_requestId], bytes32(abi.encodePacked(_randomWords,requestToToken[_requestId])));
67:         emit RequestFulfilled(_requestId, _randomWords);
68:     }

Tools Used

Manual Review

Recommended Mitigation Steps

If such a situation arises, make sure to store the randomness in a mapping for each tokenId for the respective collectionId.

mapping(uint256 collectionId => mapping(uint256 tokenId => uint256[] randomWords));

Use the randomness stored now to set the hash of the tokenId. In order to set the tokenHash in such an emergency situation, there is no other way but to also allow the admins to call the setTokenHash() function.

Solution (change on Line 311):

File: NextGenCore.sol
311:     function setTokenHash(uint256 _collectionID, uint256 _mintIndex, bytes32 _hash) external FunctionAdminRequired(this.setTokenHash.selector) {
312:         require(msg.sender == collectionAdditionalData[_collectionID].randomizerContract);
313:         require(tokenToHash[_mintIndex] == 0x0000000000000000000000000000000000000000000000000000000000000000);
314:         tokenToHash[_mintIndex] = _hash;
315:     }

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-sponsor]

a2rocket (sponsor) disputed

[a2rocket]

We are aware of this and also posted it several times on the channel as well as on the contest page. The randomizer contracts will have the funds needed.

[141345]

related to #1464

[alex-ppg]

The Warden specifies that the RandomizerRNG and RandomizerVRF oracles may lack the necessary funds to perform their oracle queries.

Per the Sponsor throughout multiple submissions as well as the contest's README.md, the contracts are accepted as being sufficiently funded at all times.

Should the need arise, administrative functions are present that permit the system to "recover" from an insufficiently funded state by resetting the randomizers, re-invoking setTokenHash via special randomizer implementations, and so on.

As an addendum for duplicated findings that relate to the payment being insufficient for the call, the payment can be reconfigured by the administrators of the contract and it is safe to assume that the NextGen team will maintain the oracles properly.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid