TangibleTNFT / baskets-foundry

baskets-foundry
2 stars 0 forks source link

[BVC-01M] Non-Compliance w/ Checks-Effects-Interactions Pattern #29

Closed chasebrownn closed 9 months ago

chasebrownn commented 9 months ago

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:

/**
 * @notice This method is the vrf callback function. Gelato will respond with our random word by calling this method.
 * @dev    Only executable by the vrf coordinator contract.
 *         Will respond to the requesting basket with the random number.
 * @param _requestId unique request identifier given to us by Gelato.
 * @param _randomness array of random numbers requested via makeRequestForRandomWords.
 */
function _fulfillRandomness(uint256 _randomness, uint256 _requestId, bytes memory) internal override {
    address basket = requestTracker[_requestId];
    // respond to the basket contract requesting entropy with it's random number.
    IBasket(basket).fulfillRandomSeed(_randomness);

    delete requestTracker[_requestId];
    delete outstandingRequest[basket];

    emit RequestFulfilled(_requestId, basket);
}

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.

chasebrownn commented 9 months ago

Fixed