Omniscia Tangible Audit
BasketsVrfConsumer Manual Review Findings
BasketsVrfConsumer Manual Review Findings
BVC-01M: Non-Compliance w/ Checks-Effects-Interactions Pattern
Type | Severity | Location |
---|---|---|
Language Specific | BasketsVrfConsumer.sol:L123, L125-L126 |
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.