First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`totalFees` can overflow causing wrong protocol fee calculation

Nocturnus

Summary

In this raffle protocol, each player contributes an entrance fee of 1e18 wei. The protocol's share is calculated based on the totalFees variable.

Vulnerability Details

The vulnerability lies in the usage of the uint64 data type for the totalFees variable, which is utilized in the selectWinner() function, as illustrated by the formula:

uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);

uint64 is used for totalFees as a variable and also same is used in selectWinner() function. Depending on the number of players who enter the raffle there is a risk of overflow.

Impact

Due to the use of uint64 for the totalFees variable, there is a risk of overflow depending on the number of players participating in the raffle. If the fee value surpasses the maximum value of uint64 (18446744073709551615), the calculation will yield an incorrect result.

Let's calculate what the maximum totalAmountCollected can be:

uint256 fee = (totalAmountCollected * 20) / 100;
18446744073709551615 = (totalAmountCollected * 20) / 100
totalAmountCollected is in wei. Let's divide both side of the equation by 1e18

18.44 = (totalAmountCollected in ether) * 20 /100
If the number of players exceeds 92 the protocol fee will be miscalculated.

Given that the maximum integer value of uint64 is 18446744073709551615, any situation where the fee value in the formula exceeds this maximum value will result in miscalculations. As calculated above, if the number of players surpasses 92, the protocol fee will be incorrectly calculated.

To prove this vulnerability, the provided Proof of Concept (PoC) demonstrates a scenario with 93 players, showcasing how the totalFees calculation would exceed the maximum value of uint64. The resulting miscalculation of the totalFees is evident from the PoC
Below is the PoC used to prove:

function testFail93players() public {
uint256 noOfPlayers = 93;
address[] memory players = new address[](noOfPlayers);
for (uint256 i = 1; i < noOfPlayers; i++) {
players[i] = vm.addr(i);
}
puppyRaffle.enterRaffle{value: entranceFee * noOfPlayers}(players);
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
puppyRaffle.selectWinner();
uint256 totalFeesAccordingToCalculation = (noOfPlayers * 1e18 * 20) / 100;
uint256 uint64OfTotalFeesAccordingToCalculation = uint64(totalFeesAccordingToCalculation);
uint256 totalFeesInTheContract = puppyRaffle.totalFees();
console.log("totalFees in the contract", totalFeesInTheContract);
console.log("totalFees should be", totalFeesAccordingToCalculation);
console.log("uint64 of above", uint64OfTotalFeesAccordingToCalculation);
console.log("====================================");
assertEq(totalFeesInTheContract, totalFeesAccordingToCalculation);
}

Tools Used

  • Manual code review

  • Foundry

Recommendations

To mitigate this issue, it is recommended to use the uint256 data type instead of uint64 for variables such as totalFees. This adjustment would prevent potential overflow problems and ensure the accurate calculation of fees, even for larger numbers of participants in the raffle.

Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

overflow-uint64

Support

FAQs

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