Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

unsafe cast on fees

Root + Impact

Description

PuppyRaffle::selectWinner involve calculating 20% from the total participant fees as fees treasury, the rest (80%) will be sent to the winner.

function selectWinner() external {
// ...
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
@>>> totalFees = totalFees + uint64(fee);
// ...
}

i notice that the fee is cast to uint64. the max value of uint64 is 18,446,744,073,709,551,615 (~18 ETH). if the participant is large enough that the fee exceeding the max value of uint64, it will result in overflow, and silently truncates the value back to 0 and start counting, causes the fee treasury to receive less than it should.

Risk

Likelihood: High/Medium

  • when the total participants are high enough that the 20% of the total deposited exceeding the max value of uint64, overflow occurs

Impact: High/Medium

  • when overflow occurs, total fees will gets truncated to 0, and start counting from here, causing the fee treasury to receive less fees than it should

Proof of Concept

assuming the participant fee is 1 ETH. a total of at least 93 participants are required to trigger this bug

function test_fee_truncation() public {
// prepare participants
address[] memory participants = new address[](93);
for (uint256 i = 0; i < participants.length; i++) {
participants[i] = address(uint160(i + 100));
}
// participants enter raffle
puppyRaffle.enterRaffle{value: entranceFee * participants.length}(participants);
// fast forward time to after raffle duration
vm.warp(block.timestamp + duration + 1 seconds);
// select winner to accumulate fees
puppyRaffle.selectWinner();
uint fees = puppyRaffle.totalFees();
console.log("Total Fees Collected (uint256):", fees);
}

create a new test file, paste the above PoC into the newly created test file, and run it.

below is the output result from the test:

[PASS] test_fee_truncation() (gas: 4835632)
Logs:
Total Fees Collected (uint256): 153255926290448384

the output log has stated a total of ~0.15 ETH has been collected. however, the entrance fee per one participant is 1 ETH!! lets see the actual amount that the fee treasury should receive using the python terminal as shown below:

>>> participantFee = 1e18
>>> totalParticipant = 93
>>> actualTotalFees = participantFee * totalParticipant * 20 / 100
>>> print(actualTotalFees)
1.86e+19

the actual total fees the treasury should receive is ~18.6 ETH, while the current cast has cause the total fees to be reduced to ~0.15 ETH !!

Recommended Mitigation

avoid casting to small data while calculating fees

function selectWinner() external {
// ...
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
// ...
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 6 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!