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

No Validation for Excess ETH in claimThrone Function

samuelsmith442

Description

The Last Man Standing game has a minor vulnerability in its claimThrone() function where there is no validation or notification for excess ETH sent by users. If a player sends more ETH than the required claim fee, the excess amount is silently added to the pot without any notification to the user.

Expected Behavior

When a player sends more ETH than the required claim fee, the contract should either:

  1. Return the excess ETH to the sender, or

  2. Clearly notify the user that the excess amount will be added to the pot

Actual Behavior

The contract silently accepts any excess ETH and adds it to the pot without notifying the user. This could lead to users accidentally overpaying without realizing it.

Root Cause

The vulnerability exists in the claimThrone() function where it only checks if the sent amount is greater than or equal to the required fee, but doesn't handle excess amounts:

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// ... rest of the function ...
uint256 sentAmount = msg.value;
// ... calculations using sentAmount ...
}

The function uses >= in the require statement, allowing for excess ETH, but doesn't provide any mechanism to handle or notify about the excess amount.

Risk Assessment

Impact

The impact is low because:

  1. It doesn't affect the core functionality of the contract

  2. The excess ETH isn't lost (it goes to the pot)

  3. Users can still participate in the game as intended

Likelihood

The likelihood is medium because:

  1. Users might not always send the exact claim fee amount

  2. The UI might not enforce exact payments

  3. Users might intentionally send more to increase the pot

Proof of Concept

The issue can be verified by examining the claimThrone() function in Game.sol. There is no mechanism to handle or notify about excess ETH sent by users.

Recommended Mitigation

Add a check for excess ETH and emit an event to notify the user:

// Before: Vulnerable code
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// ... rest of the function ...
}
​
// After: Fixed code
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// Check for excess ETH
if (msg.value > claimFee) {
emit ExcessEthReceived(msg.sender, msg.value - claimFee);
}
// ... rest of the function ...
}
​
// Add a new event
event ExcessEthReceived(address indexed sender, uint256 excessAmount);

Explanation

The fix adds a check for excess ETH and emits an event to notify the user when they send more than the required claim fee. This improves transparency and user experience without changing the core functionality of the contract.

Updates

Appeal created

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