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

[L-2] CEI pattern not followed in withdrawWinnings

wolf_kalp

Root + Impact

[L-2] CEI pattern not followed in withdrawWinnings

Description

The withdrawWinnings function updates the state after making the external call to msg.sender. This
violates the Checks-Effects-Interactions (CEI) pattern, which is a standard best practice in Solidity to
prevent reentrancy issues and unexpected behavior.

While the function uses nonReentrant, following CEI adds defense-in-depth and makes the code more robust and auditable.

Impact:

1.In general, reentrancy is prevented by nonReentrant, so this is not a direct vulnerability.

2.However, future maintainers may remove the nonReentrant modifier or introduce new logic that becomes vulnerable.

3.Also, it increases the cognitive load of reasoning about state safety during audits or upgrades.

Proof of Concept

function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
​
// Interaction before effect
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
​
// State update after external call
pendingWinnings[msg.sender] = 0;
​
emit WinningsWithdrawn(msg.sender, amount);
}
​

Recommended Mitigation

Update the state before the external call:

​
function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
​
// Effect before interaction
pendingWinnings[msg.sender] = 0;
​
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
​
emit WinningsWithdrawn(msg.sender, amount);
}
​
​
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!