Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

[H-2] No refund after overpaying ETH for claiming throne in `Game::claimThrone()`

Root Cause

The Game::claimThrone() function lacks logic to validate or refund excess ETH sent by the caller. It only checks if the sent amount is >= claimFee() but does not enforce exact match or return the excess.


Description

The Game::claimThrone() function does not check if the user has overpaid. Any ETH sent above the Game::claimFee is silently accepted and added to the pot, which may not be the intended user experience.

This could confuse users and lead to accidental overpayments. If this behavior is not intentional, it should be mitigated.

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// @audit Corrected the require statement logic for testing further.
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// Defensive check to ensure platformFee doesn't exceed available amount after previousKingPayout
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
// Remaining amount goes to the pot
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
// Update game state
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
// Increase the claim fee for the next player
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}

Impact

  • User Overpayment: Users might mistakenly send more ETH than required, expecting a refund.

  • Poor UX: Lack of feedback or refund can lead to user dissatisfaction.

  • Implied Griefing: A malicious dApp frontend or social engineering could trick users into overpaying.

  • Accounting Confusion: Pot balance may include ETH not intentionally contributed, complicating audits or payouts.


Proof of Concept (PoC)

function test_NoLogicForRefundOfExcessEthDeposited() public {
console2.log("Balance of Player1 before claiming:", player1.balance);
vm.startPrank(player1);
uint256 excess = 5 ether;
game.claimThrone{value: excess}(); // Overpaying claim fee
vm.stopPrank();
console2.log("Balance of Player1 after claiming:", player1.balance);
assertEq(game.currentKing(), player1, "Player1 should be current king");
assertGt(game.pot(), game.claimFee(), "Pot should include excess ETH"); // Confirm excess added to pot
}

Recommended Mitigation:

Option 1: Enforce Exact ETH Match

Require the sender to send exactly the required fee:

require(msg.value == claimFee(), "Game: Exact claim fee required");

Pros:

  • Clean accounting pot only grows from actual fees.

  • Prevents accidental overpayments.

  • More predictable for off-chain UIs or integrations.

Cons:

  • Might cause unnecessary revert if the user accidentally overpays.

  • Can break composability with contracts that can't control exact ETH amounts.

  • Breaks with sendValue() patterns or fallback-based UX.


Option 2: Accept overpayments but refund the difference:

uint256 fee = claimFee();
require(msg.value >= fee, "Game: Insufficient claim fee");
uint256 excess = msg.value - fee;
if (excess > 0) {
(bool success, ) = msg.sender.call{value: excess}("");
require(success, "Game: Refund failed");
}

Pros:

  • Flexible UX users can overpay without losing funds.

  • Safer for integrations, wallets, or bundlers.

  • Maintains predictable pot growth.

Cons:

  • Adds external call risk (reentrancy-safe in this case since state updated before call, but still needs attention).

  • Refunds might fail if msg.sender is a contract with fallback issues (can be mitigated with pull-based refunds if needed).


Severity: High

  • Silent loss of funds for users who accidentally overpay.

  • No event logs for excess ETH tracking or debugging is difficult.

  • Potential griefing vector: An attacker can inflate the pot size by repeatedly overpaying, making reclaiming economically infeasible for others (soft DoS).

  • Trust issues for wallet integrations or relayers expecting precise fee mechanics.

Updates

Appeal created

inallhonesty Lead Judge 9 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Overpayment not refunded, included in pot, but not in claim fee

teejay4life Auditor
9 days ago
nkh000 Auditor
9 days ago
0x00t1 Auditor
9 days ago
_gakarot Submitter
8 days ago
_gakarot Submitter
8 days ago
_gakarot Submitter
8 days ago
eagerpanda582 Auditor
8 days ago
_gakarot Submitter
8 days ago
inallhonesty Lead Judge
6 days ago
inallhonesty Lead Judge 5 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Overpayment not refunded, included in pot, but not in claim fee

Support

FAQs

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