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

Lack of Game-End Check in `withdrawWinnings()` Allows Premature Withdrawals

seenu1947

Lack of Game-End Check in withdrawWinnings() Allows Premature Withdrawals

Description

  • Players should only be allowed to withdraw winnings after the game has concluded and the winner has been officially declared via declareWinner().

  • The withdrawWinnings() function allows players to withdraw any available pendingWinnings even before the game has ended, since it doesn't check whether gameEnded is true. This breaks the game flow — players could withdraw rewards that haven't been fully finalized, especially if the contract logic is later changed to assign rewards mid-game (e.g., on claimThrone()).

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:

  • A player receives non-zero pendingWinnings during an early phase (e.g., due to future bugs or reward changes).

  • They immediately withdraw it before the game ends, bypassing intended restrictions.

  • No protections stop this today.

Impact:

  • Game reward logic becomes inconsistent.

  • In future extensions (like partial reward pre-allocation), this opens up premature extraction and economic exploits.

  • Reduces trust and fairness of the game.

Proof of Concept

// Imagine this scenario inside the Game contract:
​
// Assume the game is still ongoing
gameEnded = false;
​
// For some reason (a future feature, a bug, or manual call),
// a user gets winnings assigned before the game ends
pendingWinnings[attacker] = 5 ether;
​
// Now the attacker calls withdrawWinnings()
function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
​
// This check passes since amount = 5 ether
require(amount > 0, "Game: No winnings to withdraw.");
​
// Even though the game is still active, the transfer proceeds
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
​
// Funds are gone, and the balance is cleared
pendingWinnings[msg.sender] = 0;
​
emit WinningsWithdrawn(msg.sender, amount);
}
​
// No check for `gameEnded` means anyone with a non-zero pendingWinnings can withdraw early
​

Recommended Mitigation

function withdrawWinnings() external nonReentrant {
+ require(gameEnded, "Game: Cannot withdraw before the game ends.");
​
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);
}
​
Or, if the contract might allow **some early payouts** in the future:
​
require(gameEnded || earlyWithdrawAllowed[msg.sender], "Game: Not eligible to withdraw yet.");
Updates

Appeal created

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
seenu1947 Submitter
about 1 month ago
inallhonesty Lead Judge
about 1 month ago
inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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