Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Integer overflow in totalFees Causes Permanent Loss of Protocol Fees

georgibuilds

Root + Impact

The `totalFees` variable uses `uint64` which has a maximum value of ~18.4 ETH. After approximately 10 raffle rounds with normal participation, accumulated fees exceed this limit causing integer overflow. The overflowed value becomes incorrect, permanently preventing fee withdrawal as the contract's balance check fails.

Description

  • The protocol uses uint64 to track accumulated fees, which overflows after moderate usage:

contract PuppyRaffle is ERC721, Ownable {
address public feeAddress;
uint64 public totalFees = 0; // @audit Max ~18.4 ETH, overflows easily
function selectWinner() external {
// ...
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee); // @audit Unsafe cast causes overflow
// ...
}
function withdrawFees() external {
// @audit Check fails after overflow - fees locked forever
require(address(this).balance == uint256(totalFees), "Players active!");
// ...
}
}
```
```
Overflow scenario with 90 players per round at 1 ETH entrance fee:
- Round 1: 18 ETH fees → totalFees = 18 ETH
- Round 2: 18 ETH fees → totalFees should be 36 ETH
- Actual: uint64 overflow → totalFees = ~17.55 ETH (wraps around)
​
Contract has 36 ETH but totalFees shows 17.55 ETH. withdrawFees() check: 36 == 17.55? Fails permanently.

Risk

Likelihood:

  • Occurs after ~10 normal raffle rounds (90 players, 1 ETH fee)

  • Higher entrance fees or more players accelerate overflow

Inevitable with continued protocol usage

Impact:

  • Permanent loss of all accumulated protocol fees

Owner cannot recover funds (no alternative withdrawal mechanism)

Protocol becomes economically unviable for owner after overflow

Only affects owner fees, not player deposits

Proof of Concept

// Run: forge test --mt testTotalFeesOverflow -vv // Result: // Round 1 - totalFees: 18 ETH // Round 2 - totalFees: 17.55 ETH (wrapped around from 36 ETH) // Expected fees: 36 ETH // Actual fees: 17.55 ETH // Owner CANNOT withdraw fees - funds are locked!

// Add to test/PuppyRaffleTest.t.sol
​
function testTotalFeesOverflow() public {
uint256 playersPerRound = 90;
// Run 2 raffle rounds
for (uint256 round = 0; round < 2; round++) {
address[] memory players = new address[](playersPerRound);
for (uint256 i = 0; i < playersPerRound; i++) {
players[i] = address(uint160(i + 1 + (round * playersPerRound)));
}
puppyRaffle.enterRaffle{value: entranceFee * playersPerRound}(players);
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
puppyRaffle.selectWinner();
console.log("Round", round + 1, "- totalFees:", puppyRaffle.totalFees());
}
uint256 actualFees = puppyRaffle.totalFees();
uint256 expectedFees = 2 * 90 * entranceFee * 20 / 100; // 36 ETH
console.log("Expected fees:", expectedFees);
console.log("Actual fees (after overflow):", actualFees);
// Verify overflow occurred
assertLt(actualFees, expectedFees, "totalFees should have overflowed");
// Verify withdrawal fails - fees permanently locked
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
console.log("Owner CANNOT withdraw fees - funds are locked!");
}
​
// Run: forge test --mt testTotalFeesOverflow -vv
// Result:
// Round 1 - totalFees: 18 ETH
// Round 2 - totalFees: 17.55 ETH (wrapped around from 36 ETH)
// Expected fees: 36 ETH
// Actual fees: 17.55 ETH
// Owner CANNOT withdraw fees - funds are locked!

Recommended Mitigation

uint256 max value: ~115 octillion ETH (effectively unlimited)

  • Eliminates overflow risk entirely

  • No performance or gas penalty

  • Simple one-line change

contract PuppyRaffle is ERC721, Ownable {
address public feeAddress;
- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
function selectWinner() external {
// ...
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
// ...
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 23 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-05] Typecasting from uint256 to uint64 in PuppyRaffle.selectWinner() May Lead to Overflow and Incorrect Fee Calculation

## Description ## Vulnerability Details The type conversion from uint256 to uint64 in the expression 'totalFees = totalFees + uint64(fee)' may potentially cause overflow problems if the 'fee' exceeds the maximum value that a uint64 can accommodate (2^64 - 1). ```javascript totalFees = totalFees + uint64(fee); ``` ## POC <details> <summary>Code</summary> ```javascript function testOverflow() public { uint256 initialBalance = address(puppyRaffle).balance; // This value is greater than the maximum value a uint64 can hold uint256 fee = 2**64; // Send ether to the contract (bool success, ) = address(puppyRaffle).call{value: fee}(""); assertTrue(success); uint256 finalBalance = address(puppyRaffle).balance; // Check if the contract's balance increased by the expected amount assertEq(finalBalance, initialBalance + fee); } ``` </details> In this test, assertTrue(success) checks if the ether was successfully sent to the contract, and assertEq(finalBalance, initialBalance + fee) checks if the contract's balance increased by the expected amount. If the balance didn't increase as expected, it could indicate an overflow. ## Impact This could consequently lead to inaccuracies in the computation of 'totalFees'. ## Recommendations To resolve this issue, you should change the data type of `totalFees` from `uint64` to `uint256`. This will prevent any potential overflow issues, as `uint256` can accommodate much larger numbers than `uint64`. Here's how you can do it: Change the declaration of `totalFees` from: ```javascript uint64 public totalFees = 0; ``` to: ```jasvascript uint256 public totalFees = 0; ``` And update the line where `totalFees` is updated from: ```diff - totalFees = totalFees + uint64(fee); + totalFees = totalFees + fee; ``` This way, you ensure that the data types are consistent and can handle the range of values that your contract may encounter.

Support

FAQs

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

Give us feedback!