RandomizerVRF.sol: malicious function admin can set invalid keyhash to DoS randomness generation

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/RandomizerVRF.sol#L79-L82
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/RandomizerVRF.sol#L52-L63

Vulnerability details

Impact

Malicious function admin can set an invalid keyhash so that randomness generation will be DoSed. The idea is similar to this finding: code-423n4/2022-12-forgeries-findings#101

Proof of Concept

Malicious function admin can call updatecallbackGasLimitAndkeyHash() to set an invalid keyhash:

    function updatecallbackGasLimitAndkeyHash(uint32 _callbackGasLimit, bytes32 _keyHash) public FunctionAdminRequired(this.updatecallbackGasLimitAndkeyHash.selector){
        callbackGasLimit = _callbackGasLimit;
        keyHash = _keyHash;
    }

This keyhash will be used to send randomness generation requests to Chainlink VRF:

    function requestRandomWords(uint256 tokenid) public {
        require(msg.sender == gencore);
        uint256 requestId = COORDINATOR.requestRandomWords(
            keyHash,
            s_subscriptionId,
            requestConfirmations,
            callbackGasLimit,  
            numWords
        );
        tokenToRequest[tokenid] = requestId;
        requestToToken[requestId] = tokenid;
    }

Since there is no check on validity of keyhash, calls to requestRandomWords() will always revert if keyHash is invalid, hence randomness generation will be DoSed.

Tools Used

Manual review

Recommended Mitigation Steps

Validate keyhash inside updatecallbackGasLimitAndkeyHash(). Compare the new keyhash with a whitelisted possible keyhashes.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-sponsor]

a2rocket (sponsor) disputed

[a2rocket]

admins are trusted roles.

[alex-ppg]

The Warden specifies that an administrator can set an invalid key hash for Chainlink requests, falling under the category of reckless administrator mistakes per the relevant SC ruling as there is no incentive for the administrator to do so. Such a submission is relevant in an Analysis report and irrelevant when assessing actual vulnerabilities of the protocol.

The precedence cited is of a project over a year ago when Analysis reports were not an option / dedicated section of the fund pool. As such, I do not consider the citation valid and maintain my "invalid" judgment.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid