Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

CEI Pattern Violation in withdrawWinnings()

CEI Pattern Violation in withdrawWinnings()

Description

  • The function violates Checks-Effects-Interactions (CEI) pattern but is protected by nonReentrant modifier.

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

Risk

Likelihood:

  • Med: Though the pattern has been violated buy the impact is non-critical

Impact:

  • Since the function is already protected by nonReentrant guard, so the attacker can not reenter the `withdrawWinnings` function, causing low impact.

Recommended Mitigation

Follow CEI pattern for code clarity and defense-in-depth.

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

Appeal created

inallhonesty Lead Judge about 2 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.