sherlock-audit / 2023-10-looksrare-judging

6 stars 6 forks source link

836541 - Gas Limit for VRF's callbacks should be changeable #68

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

836541

medium

Gas Limit for VRF's callbacks should be changeable

Summary

The protocol requests for randomness via '_requestForRandomness' with a limit of "2_500_000" gas for the "fulfillRandomWords" callback. However, hardcoding a gas limit can be risky because gas prices are subject to EVM changes. Specially for VRF, which the docs states that the callback must not revert (https://docs.chain.link/vrf/v2/security#fulfillrandomwords-must-not-revert) since VRF never tries to call it again.

Vulnerability Detail

  1. In _requestForRandomness(), the callbackGasLimit is set: callbackGasLimit: uint32(2_500_000)
    function _requestForRandomness() private {
        uint256 requestId = VRF_COORDINATOR.requestRandomWords({
            keyHash: KEY_HASH,
            subId: SUBSCRIPTION_ID,
            minimumRequestConfirmations: uint16(3),
            callbackGasLimit: uint32(2_500_000),
            numWords: uint32(1)
        });
  2. The gas shouldn't be hardcoded, because if it isn't enough and a revert happen, Chainlink states that VRF don't call the callback again.
  3. In this scenario, protocol has a startNewRound function to re-request randomness. However, the method shouldn't exist as stated in my other report.

Impact

  1. Likelihood: Low, it needs EVM updates and network congestions to happen.
  2. Severity: High, because VRF don't try to call the callback again and ideally startNewRound for re-requests shouldn't exist as stated in my other report, which would make reverts in the callback a denial of service.
  3. Impact: Low Likelihood + High Severity = Medium Impact.

Code Snippet

https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L1298

        callbackGasLimit: uint32(2_500_000),

Tool used

Manual Review

Recommendation

Add an admin function that allows changing the callbackGasLimit.