sherlock-audit / 2024-01-looksrare-judging

3 stars 0 forks source link

Anubis - Unchecked Return Value in **cancelAfterRandomnessRequest** #115

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Anubis

medium

Unchecked Return Value in cancelAfterRandomnessRequest

Summary

The cancelAfterRandomnessRequest function in the smart contract does not verify the return value of external calls to the Chainlink VRF coordinator. This neglect could lead to unhandled failures or incorrect assumptions about the state of the system.

Vulnerability Detail

In cancelAfterRandomnessRequest, an external call is made to request randomness from the Chainlink VRF coordinator. However, the return value of this call is not checked, which is a critical oversight. This can result in the smart contract behaving unpredictably or incorrectly if the external call fails, but the contract's state assumes success.

Impact

A failure in handling the external call's return value could lead to serious issues like state inconsistencies, potential vulnerabilities to attacks, or financial loss. This undermines the reliability and security of the contract.

Code Snippet

In cancelAfterRandomnessRequest, the following code snippet illustrates the issue: https://github.com/sherlock-audit/2024-01-looksrare/blob/main/contracts-yolo/contracts/YoloV2.sol#L463-L467

In the code snippets above, the low-level calls do not check the return value, assuming that the transfers will always succeed.

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, it's crucial to check the return values of external calls. Implement a robust error handling mechanism to manage failed calls appropriately. For this specific function, a require statement should be added to ensure that the external call to the Chainlink VRF coordinator completes successfully. If the call fails, the transaction should be reverted to maintain contract state integrity.

The exact fix for the cancelAfterRandomnessRequest function is as follows:

function cancelAfterRandomnessRequest() external nonReentrant {
    // ... [omitted for brevity]
    round.status = RoundStatus.Cancelled;
    emit RoundStatusUpdated(roundId, RoundStatus.Cancelled);

    // Change _startRound to return a boolean status.
    bool success = _startRound({_roundsCount: roundId});
    require(success, "External call to Chainlink VRF failed");
}

In the _startRound function, ensure it returns a boolean indicating the success status of the operation. This modification ensures that the function execution halts if the external call is unsuccessful, preserving the contract's consistent state and preventing potential vulnerabilities.

nevillehuang commented 9 months ago

Invalid, explicit check is not required, if the function fails, it will simply revert, given there is no low level call executed