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

State Update After External Call Can Lead to Future Reentrancy Risks

seenu1947

State Update After External Call Can Lead to Future Reentrancy Risks

Description

  • In Solidity, even with a nonReentrant modifier, it is a best practice to update contract state before making external calls, such as .call{value: amount}. This protects against reentrancy bugs, especially if the modifier is removed, modified, or bypassed later.

  • In the current withdrawWinnings() function, the contract makes an external call to msg.sender before updating the user's pendingWinnings balance. This creates a vulnerable structure where the control flow is handed to an external address while the internal state is not yet cleaned up.

function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
​
// @> External call happens BEFORE state update
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
​
// @> State is updated too late — should be before the call
pendingWinnings[msg.sender] = 0;
​
emit WinningsWithdrawn(msg.sender, amount);
}

Risk

Likelihood:

  • This is not exploitable with the current nonReentrant in place.

  • However, it's a commonly misused pattern that opens risk during refactoring, modifier removal, inheritance override, or future multi-function interactions.

Impact:

  • If a future version introduces a state update after a reentrant-safe assumption is broken, a reentrancy exploit could occur.

  • Trust and safety assumptions of the contract's withdrawal pattern could be violated.

Proof of Concept

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);
}

Recommended Mitigation

function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
​
+ // State is cleared BEFORE external 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 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.