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

User can send more ETH than claimFee

neomartis

User can send extra ETH than claimFee

Description

  • User can send more ETH than the claimFee

  • This will all be added to the pot

As per documentation:

* Can `claimThrone()` by sending the required `claimFee`.

It is not specified in the requirements what to do with leftover ETH.Any extra ETH could be considered a tip, this can either be for:

  • The owner/contract

  • The winner (current behaviour of the game)

>@ require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");

Risk

Likelihood: Low

  • This will adjust the winning pot-size, either the extra value is for the winner (currently), or for the deployer/contract

Impact: Low

  • No big impact, although it should be clarified / intended behaviour

Consider the claimThrone function:

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// ...
uint256 sentAmount = msg.value;
// ...
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// ...
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;

As you see Both

  • platformFeesBalance

  • pot

Gets updated based on msg.value, not based on claimFee, meaning that all the ETH is being sent to the pot and platform fees.

Recommended Mitigation

To fix this, it should be specified in the requirements what to do with leftover ETH.
Either:

  • The owner/contract

  • The winner (current behaviour of the game)

Alternatively the requirement check could be stricter, to disallow sending more ETH than the claimFee

option 1

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// ...
uint256 sentAmount = msg.value;
// ...
- currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
+ currentPlatformFee = (claimFee * platformFeePercentage) / 100;
// ...
- platformFeesBalance = platformFeesBalance + currentPlatformFee;
+ platformFeesBalance = platformFeesBalance + currentPlatformFee + (sentAmount - claimFee);
​
// ...
- amountToPot = sentAmount - currentPlatformFee;
+ amountToPot = claimFee - currentPlatformFee;
pot = pot + amountToPot;

option 2

function claimThrone() external payable gameNotEnded nonReentrant {
- require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
+ require(msg.value == claimFee, "Game: ETH is not equal to claimFee.");
Updates

Appeal created

inallhonesty Lead Judge 4 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.

Give us feedback!