Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

No Refund for Excess ETH Sent

mohankrishkotte

No Refund for Excess ETH Sent causing, Players lose excess ETH sent above claim fee

Description

  • Players may accidentally send more ETH than the required claim fee when calling claimThrone().

  • The contract does not refund the excess amount; all sent ETH is added to the pot and platform fees.

// src/Game.sol
function claimThrone() external payable gameNotEnded nonReentrant {
@> require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne."); //@> No refund for excess
...
}

Risk

Likelihood:

  • This will occur whenever a player overpays the claim fee.

  • Users may lose funds due to mistakes.

Impact:

  • Loss of user funds.

  • Poor user experience and reduced trust.

Proof of Concept

  • The function checks: require(msg.value >= claimFee, ...)

  • If you send less: It fails (reverts).

  • If you send more: All ETH (msg.value) is accepted. The contract proceeds to process the claim as if only the claimFee had been sent.

  • But: There is no logic to refund msg.value - claimFee (the "overpaid" amount).

  • All the ETH received is distributed among platform fees and the pot per the function's logic; the player cannot recover any excess ETH sent by mistake.

function testNoRefundForExcessETH() public {
// Player1 initiates a claim; current claim fee is, say, 0.1 ether
vm.startPrank(player1);
​
// Intentionally send extra ETH: claimFee + 0.5 ether extra
uint256 overpayAmount = INITIAL_CLAIM_FEE + 0.5 ether;
uint256 player1BalanceBefore = player1.balance;
​
game.claimThrone{value: overpayAmount}();
​
vm.stopPrank();
​
// After the transaction, check the actual balance drop for player1
uint256 player1BalanceAfter = player1.balance;
​
// The balance decrease should be exactly overpayAmount:
assertEq(player1BalanceBefore - player1BalanceAfter, overpayAmount, "All ETH sent was deducted, no refund");
​
// There is NO code refunding excess: so player1 loses the extra 0.5 ether, which goes to pot/platform fees
}
​

Recommended Mitigation

Add explicit logic in claimThrone() to refund any amount sent above the required claim fee.

​
+ function claimThrone() external payable gameNotEnded nonReentrant {
+ require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
​
+ // Refund any excess ETH to the sender
+ uint256 excess = msg.value - claimFee;
+ if (excess > 0) {
+ payable(msg.sender).transfer(excess);
+ }
​
+ // Use 'claimFee' only for the game logic
+ uint256 sentAmount = claimFee;
​
+ // ... (rest of your logic using sentAmount)
+}
​
Updates

Appeal created

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