Submission Details
Severity:
Game::claimThrone() Missing logic send small portion of the new claim fee for previous King.
Root + Impact
Description
When there are new King claim throne, If there's a previous king, a small portion of the new claim fee is sent to them. But function
Game::claimThrone()is missing that logic.
// Root cause in the codebase with @> marks to highlight the relevant section
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;
@> Missing calculate payout for previous King
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
Risk
Likelihood:
Reason 1: Everytime when a player create transaction
claimThrone()
Impact:
-
Impact 1: Its missing that logic, not as described in the document.
-
Impact 2: Previous King will receive less than expected.
Proof of Concept
-
pendingWinningsstate is updated only one time when declareWinner -
and it add
potforcurrentKing, not previous King
function declareWinner() external gameNotEnded {
...
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
...
}
Recommended Mitigation
contract Game is Ownable {
+ uint256 previousKingPayoutPercentage;
+ function updatePreviousKingPayoutPercentage(uint256 percentage) external onlyOwner isValidPercentage(percentage) {
+ previousKingPayoutPercentage = percentage;
+
+ emit UpdatePreviousKingPayoutPercentage(percentage);
+ }
function claimThrone() external payable gameNotEnded nonReentrant {
...
+ currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
- uint256 previousKingPayout = 0;
+ uint256 previousKingPayout = currentPlatformFee * previousKingPayoutPercentage / 100;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
- // Calculate platform fee
- currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
...
- platformFeesBalance = platformFeesBalance + currentPlatformFee;
+ platformFeesBalance = platformFeesBalance + currentPlatformFee - previousKingPayout;
amountToPot = sentAmount - (currentPlatformFee);
pot = pot + amountToPot;
+ address previousKing = currentKing;
currentKing = msg.sender;
+ pendingWinnings[previousKing] = pendingWinnings[previousKing] + previousKingPayout;
...
}
}
Updates
Appeal created
inallhonesty about 2 months ago
Submission Judgement Published Assigned finding tags:
Missing Previous King Payout Functionality
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.