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

Current claim fee model based on the previous fee fails to reward higher contributions.

soupy

Summary

The current implementation calculates the new claim fee by applying a fixed feeIncreasePercentage to the previous claim fee, rather than basing it on the sendAmount provided by the player in the claimThrone function. As a result, any additional amount sent by the player beyond the required claim fee does not contribute to increasing the next claim fee. This reduces the strategic advantage for the player, who might aim to discourage future claims by raising the claimFee threshold.

Description

New claim fee was computed based on the addition of feeIncreasePercentage on previous claim fee instead of the sendAmount input by player in the claimThrone function. This causes the additional amount sent in by the player was not used to raise the the next claim fee to increase the chances of winning for the currentKing with the hope of no next claim due to the new high claim fee.

function claimThrone() external payable gameNotEnded nonReentrant {
<@@> require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
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
);
}

Risk

Likelihood:

  • Current implementation affects all players who send more than the minimum claim fee, they receive no strategic advantage in return. The extra amount they contribute does not help increase the next claim fee, meaning it doesn't improve their chances of retaining the position of currentKing. Instead, the additional funds go entirely toward increasing the platform's fee and the final pot value, benefits that the player may not enjoy if a new claimant emerges shortly after.

Impact:

  • Current implementation may discourage players from contributing more than the minimum claim fee, as the additional amount offers no direct advantage in retaining the throne. As a result, overall user engagement and strategic play may be affected. Players may become less willing to invest higher amounts, leading to lower platform revenue growth and a slower increase in the final pot value.

Instead switching to a model where the new claim fee is calculated based on the actual sendAmount would provide several benefits as below:

  • Incentivizes higher bids: Players are motivated to send more than the minimum, increasing their chances of retaining the throne longer.

  • Increased platform revenue: Higher sendAmount directly contributes to greater fee collection.

  • Larger final pot value: As more funds are funneled through claims, the overall pot value increases, enhancing the game’s appeal.

This change aligns player incentives with platform growth, rewarding both high bidders and the platform simultaneously.

Proof of Concept

In test/Game.t.sol, add the following test:

function test_audit_nonIncentivizingClaimFeeCalculation() public {
uint256 player1SendAmount = 1 ether;
console2.log("player1_sendAmount: ", player1SendAmount);
​
uint256 currentClaimFee = game.claimFee();
console2.log("current_claim_fee: ", currentClaimFee);
​
// player 1 makes the claim with higher amount than the required initial claim fee
vm.prank(player1);
game.claimThrone{value: player1SendAmount}();
uint256 newClaimFee = game.claimFee();
console2.log("new_claim_fee: ", game.claimFee());
​
uint256 playerExpectedNewClaimFee = currentClaimFee + (player1SendAmount * 10 / 100);
console2.log("player_expected_claim_fee: ", playerExpectedNewClaimFee);
​
// passing the assertion shows that the new claim fee is less than what the amount of player 1 expected it would be, the extra money is not factored in during the claim fee calculation
assertLt(newClaimFee, playerExpectedNewClaimFee);
}

In terminal, run forge test --match-test test_audit_nonIncentivizingClaimFeeCalculation -vvv will generate the following results:

forge test --match-test test_audit_nonIncentivizingClaimFeeCalculation -vvv
...
​
Ran 1 test for test/Game.t.sol:GameTest
[PASS] test_audit_nonIncentivizingClaimFeeCalculation() (gas: 160356)
Logs:
player1_sendAmount: 1000000000000000000
current_claim_fee: 100000000000000000
new_claim_fee: 110000000000000000
player_expected_claim_fee: 200000000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 821.33µs (89.17µs CPU time)
​
Ran 1 test suite in 116.34ms (821.33µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

The test passed showing that the next new claim fee was lesser than the amount the player expected it to be, which the extra money invested didn't help the player strategically.

Recommended Mitigation

Use sendAmount instead of claimFee when computes the next claim fee:

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
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;
​
...
​
// Increase the claim fee for the next player
- claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
+ claimFee = claimFee + (sendAmount * feeIncreasePercentage) / 100;
​
...
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.