Skip to content
This repository has been archived by the owner on May 5, 2024. It is now read-only.

cergyk - Winning agent id may be uninitialized when game is over, locking grand prize #31

Open
sherlock-admin2 opened this issue Nov 4, 2023 · 3 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 4, 2023

cergyk

high

Winning agent id may be uninitialized when game is over, locking grand prize

Summary

In the Infiltration contract, the agents mapping holds all of the agents structs, and encodes the ranking of the agents (used to determine prizes at the end of the game).

This mapping records are lazily initialized when two agents are swapped (an agent is either killed or escapes):

  • The removed agent goes to the end of currently alive agents array with the status Killed/Escaped and its agentId is set
  • The last agent of the currently alive agents array is put in place of the previously removed agent and its agentId is set

This is the only moment when the agentId of an agent record is set.

This means that if the first agent in the array ends up never being swapped, it keeps its agentId as zero, and the grand prize is unclaimable.

Vulnerability Detail

We can see in the implementation of claimGrandPrize that:
https://github.com/sherlock-audit/2023-10-looksrare/blob/main/contracts-infiltration/contracts/Infiltration.sol#L658

The field Agent.agentId of the struct is used to determine if the caller can claim. Since the id is zero, and it is and invalid id for an agent, there is no owner for it and the condition:

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

Always reverts.

Impact

The grand prize ends up locked/unclaimable by the winner

Code Snippet

Tool used

Manual Review

Recommendation

In claimGrandPrize use 1 as the default if agents[1].agentId == 0:

function claimGrandPrize() external nonReentrant {
    _assertGameOver();
    uint256 agentId = agents[1].agentId;
+   if (agentId == 0)
+       agentId = 1;
    _assertAgentOwnership(agentId);

    uint256 prizePool = gameInfo.prizePool;

    if (prizePool == 0) {
        revert NothingToClaim();
    }

    gameInfo.prizePool = 0;

    _transferETHAndWrapIfFailWithGasLimit(WETH, msg.sender, prizePool, gasleft());

    emit PrizeClaimed(agentId, address(0), prizePool);
}
@github-actions github-actions bot added High A valid High severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Nov 6, 2023
@0xhiroshi 0xhiroshi added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Nov 6, 2023
@0xhiroshi
Copy link

@sherlock-admin sherlock-admin changed the title Fun Aegean Halibut - Winning agent id may be uninitialized when game is over, locking grand prize cergyk - Winning agent id may be uninitialized when game is over, locking grand prize Nov 9, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Nov 9, 2023
@0xhiroshi
Copy link

@SergeKireev
Copy link
Collaborator

Fix LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants