Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

uint64(fee) overflow for high entranceFee / newPlayers

Summary

The uint64(fee) cast results in an overflow for significantly high PuppyRaffle::entranceFee values or a large number of PuppyRaffle::newPlayers. As a consequence, this results in an incorrect amount of PuppyRaffle::totalFee to withdrawal.

Vulnerability Details

function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
@> uint256 fee = (totalAmountCollected * 20) / 100;
@> totalFees = totalFees + uint64(fee);
uint256 tokenId = totalSupply();
...
}

Impact

The maximum value an uint64 can hold in Solidity is 2 **64 - 1 = 18446744073709551615.
The fee is calculated as 20% of the total amount collected, which is the number of players times the entrance fee (in our test: 1 ETH = 1000000000000000000 wei).
To calculate the maximum number of players that can enter the raffle before an overflow occurs, we need to solve the following equation:

totalAmountCollected * 20 / 100 <= 18446744073709551615

Substituting totalAmountCollected with numberOfPlayers * entranceFee gives us:

numberOfPlayers * entranceFee * 20 / 100 <= 18446744073709551615

Solving for numberOfPlayers gives us:

numberOfPlayers <= (18446744073709551615 * 100) / (entranceFee * 20)

Substituting entranceFee with 1000000000000000000 (1 ETH in wei) gives us:

numberOfPlayers <= (18446744073709551615 * 100) / (1000000000000000000 * 20)

Solving this equation gives us the maximum number of players that can enter the raffle before an overflow occurs:
numberOfPlayers <= 92,2.

//The totalFee will overflow for high numbers of players / high fee amount.
uint256 entranceFee = 1e18;
function testEnter93PlayersAndCheckForOverflow() public {
// Prepare 93 players
address[] memory players = new address[](93);
for (uint256 i = 0; i < 93; i++) {
players[i] = address(i + 1); // Assign unique addresses for each player
}
// Enter the raffle
puppyRaffle.enterRaffle{value: entranceFee * 93}(players);
// Check if all players have been entered
for (uint256 i = 0; i < 93; i++) {
assertEq(puppyRaffle.players(i), players[i]);
}
// Calculate the total fees that would be collected from the raffle
uint256 totalFees = entranceFee * 93 * 20 / 100;
console.log("The total fees are:", totalFees);
uint64 totalFees64 = uint64(totalFees);
console.log("The total fees are:", totalFees64);
// Check if the total fees exceed the maximum value of uint64
bool overflowOccurred = totalFees > 2 ** 64 - 1;
assertTrue(overflowOccurred, "Overflow should occur");
}

Tools Used

Manual Review

Recommendations

Defining the totalFee variable as uint256.

- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
uint256 tokenId = totalSupply();
..
}
Updates

Lead Judging Commences

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

unsafe cast of fee to uint64

Support

FAQs

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