gkrastenov - Possible blocking of the game

[sherlock-admin]

gkrastenov

medium

Possible blocking of the game

Summary

Possible blocking of the game

Vulnerability Detail

When the owner decides to start the game, he calls the startGame() function and request random word from the VRF Coordinator. If the Chainlink randomness callback does not return after 1 day, the owner can call startNewRound() to trigger a new randomness request.

 /**
     * @inheritdoc IInfiltration
     * @dev If Chainlink randomness callback does not come back after 1 day, we can call
     *      startNewRound to trigger a new randomness request.
     */
    function startGame() external onlyOwner 

Unfortunately, this is not possible because to call a new round, the condition block.number < uint256(gameInfo.currentRoundBlockNumber).unsafeAdd(BLOCKS_PER_ROUND) should be satisfied.

if (block.number < uint256(gameInfo.currentRoundBlockNumber).unsafeAdd(BLOCKS_PER_ROUND)) {
            revert TooEarlyToStartNewRound();
 }

Variable gameInfo.currentRoundBlockNumber is set to 0 by default and is updated only in fulfillRandomWords(). In this scenario, it will always be zero because the Chainlink randomness callback is delayed.

So, if the Chainlink randomness callback is delayed for more than 1 day and the owner tries to request a random word, it will fail.

Impact

In this scenario, the whole game will be blocked and it will not be possible to start. Marked this issue as Medium because it is rare for Chainlink randomness callback to have this delay, but it is not impossible.

Code Snippet

https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L594

Tool used

Manual Review

Recommendation

Update gameInfo.currentRoundBlockNumber when startGame() function is called:

gameInfo.currentRoundId = 1;
gameInfo.activeAgents = uint16(numberOfAgents);
+gameInfo.currentRoundBlockNumber = block.number
uint256 balance = address(this).balance;

[nevillehuang]

Accepted risks, see comment in #136, contract owner can invoke emergencyWithdraw() in this case.

[Czar102]

It is not a duplicate of #136.