Omniscia Tangible Audit

GelatoVRFConsumerBase Manual Review Findings

GelatoVRFConsumerBase Manual Review Findings

GVR-01M: Non-Compliance w/ Checks-Effects-Interactions Pattern

Description:

The GelatoVRFConsumerBase::fulfillRandomnessTestnet and GelatoVRFConsumerBase::fulfillRandomness functions will fulfil randomness via the GelatoVRFConsumerBase::_fulfillRandomness function before setting the requestId pending status to false, permitting a theoretical re-entrancy attack to be executed.

Example:

src/abstract/GelatoVRFConsumerBase.sol
100/// @notice Callback function used by Gelato VRF to return the random number.
101/// The randomness is derived by hashing the provided randomness with the request ID.
102/// @param randomness The random number generated by Gelato VRF.
103/// @param dataWithRound Additional data provided by Gelato VRF containing request details.
104function fulfillRandomness(
105 uint256 randomness,
106 bytes calldata dataWithRound
107) external {
108 require(msg.sender == _operator(), "only operator");
109
110 (, bytes memory data) = abi.decode(dataWithRound, (uint256, bytes));
111 (uint256 requestId, bytes memory extraData) = abi.decode(
112 data,
113 (uint256, bytes)
114 );
115
116 bytes32 requestHash = keccak256(dataWithRound);
117 bool isValidRequestHash = requestHash == requestedHash[requestId];
118
119 require(requestPending[requestId], "request fulfilled or missing");
120
121 if (isValidRequestHash) {
122 randomness = uint(
123 keccak256(
124 abi.encode(
125 randomness,
126 address(this),
127 block.chainid,
128 requestId
129 )
130 )
131 );
132
133 _fulfillRandomness(randomness, requestId, extraData);
134 requestPending[requestId] = false;
135 }
136}

Recommendation:

While the BasketsVrfConsumer::_fulfillRandomness function which in turn invokes Basket::fulfillRandomSeed does not expose a re-entrant call, it is still advisable for the code to adhere to the CEI pattern and set a request's pending status to false prior to fulfilling it.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The GelatoVRFConsumerBase::_fulfillRandomness function invocation in each of the referenced instances has been relocated after the respective requestPending entry has been deleted, ensuring that the CEI pattern is conformed to and thus addressing this exhibit's concerns.