Omniscia Tangible Audit

BasketsVrfConsumer Manual Review Findings

BasketsVrfConsumer Manual Review Findings

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

Description:

The BasketsVrfConsumer::_fulfillRandomness function will relay the _randomness to the target basket before erasing the requestTracker and outstandingRequest entries, permitting a theoretical re-entrancy attack to occur.

Example:

src/BasketsVrfConsumer.sol
113/**
114 * @notice This method is the vrf callback function. Gelato will respond with our random word by calling this method.
115 * @dev Only executable by the vrf coordinator contract.
116 * Will respond to the requesting basket with the random number.
117 * @param _requestId unique request identifier given to us by Gelato.
118 * @param _randomness array of random numbers requested via makeRequestForRandomWords.
119 */
120function _fulfillRandomness(uint256 _randomness, uint256 _requestId, bytes memory) internal override {
121 address basket = requestTracker[_requestId];
122 // respond to the basket contract requesting entropy with it's random number.
123 IBasket(basket).fulfillRandomSeed(_randomness);
124
125 delete requestTracker[_requestId];
126 delete outstandingRequest[basket];
127
128 emit RequestFulfilled(_requestId, basket);
129}

Recommendation:

While no re-entrancy can be performed as the Basket::fulfillRandomSeed function does not contain any external calls, we still advise the CEI pattern to be adhered to by deleting the relevant entries before invoking the Basket::fulfillRandomSeed function.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The Basket::fulfillRandomSeed call has been relocated after the request tracker and outstanding request entries have been deleted, ensuring the code complies with the CEI pattern and thus addressing its concerns.