Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

Out-of-Order State Update in `withdrawWinnings`

jufel

Description:
The withdrawWinnings function conducts the external ETH transfer before resetting the caller’s pendingWinnings. While the function is protected by a nonReentrant modifier, this ordering violates the Checks–Effects–Interactions pattern and could introduce reentrancy risks if the modifier is ever removed or bypassed.

Impact:

  • Reentrancy Vulnerability (if guard removed): An attacker could reenter withdrawWinnings during the external call and drain their pendingWinnings multiple times.

  • Best Practice Violation: Deviating from Checks–Effects–Interactions makes the contract more fragile and prone to future bugs if the reentrancy guard is altered.

Mitigation:
Apply Checks–Effects–Interactions by zeroing the state before the external call:

function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
+ pendingWinnings[msg.sender] = 0;
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
- pendingWinnings[msg.sender] = 0;
}
Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Game::withdrawWinnings reentrancy

We only audit the current code in scope. We cannot make speculation with respect to how this codebase will evolve in the future. For now there is a nonReentrant modifier which mitigates any reentrancy. CEI is a good practice, but it's not mandatory. Informational

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!