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

Integer Overflow in totalFees Accumulation Permanently Blocks withdrawFees()

slientj

Root + Impact

Description

Normal Behavior:
The selectWinner function accumulates 20% of the prize pool into totalFees. The withdrawFees function then requires address(this).balance == uint256(totalFees) before allowing the owner to withdraw the accumulated fees.

Specific Issue:
totalFees is declared as uint64, but the contract uses Solidity ^0.7.6, which does not include built-in overflow checks. When the accumulated fees exceed the maximum value of uint64 (18,446,744,073,709,551,615 wei, or ~18.45 ether), totalFees silently overflows and wraps around to a smaller value. This breaks the equality check in withdrawFees, permanently blocking fee withdrawa

// @> totalFees is uint64, but Solidity 0.7.x has no overflow protection
totalFees = totalFees + uint64(fee);

Risk

Likelihood:

The overflow occurs naturally as the raffle continues over many rounds. With 4 players per round and 0.8 ether added per round, it takes about 24 rounds to reach the uint64 limit. No attacker action or special permissions are required — it is a time-based risk that grows with normal usage.

Impact:

When totalFees overflows, it wraps around to a small value. The withdrawFees function requires address(this).balance == uint256(totalFees), so the equality check fails permanently. The owner can never withdraw the accumulated fees, and the funds are locked in the contract.

Proof of Concept

The test demonstrates the overflow with concrete numbers:

Step Value (wei)
uint64 maximum 18446744073709551615
totalFees before overflow 18400000000000000000
Fee added by next round 800000000000000000
Sum before uint64 wrap 19200000000000000000
Expected totalFees after wrap 753255926290448384
Contract balance before overflow 18400000000000000000
Actual totalFees after overflow 753255926290448384
Contract balance after overflow 19200000000000000000

The test accumulates fees over multiple rounds until totalFees reaches 18.4 ether. The next round adds 0.8 ether, pushing the sum to 19.2 ether, which exceeds the uint64 maximum of 18.4467 ether. As a result, totalFees silently wraps around to 0.753 ether.

At this point, the contract balance is 19.2 ether, but totalFees is only 0.753 ether. The withdrawFees function requires address(this).balance == uint256(totalFees), so the equality check fails permanently. The test confirms this with vm.expectRevert, proving that the owner can never withdraw the accumulated fees.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console2 as console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../../../src/PuppyRaffle.sol";
​
contract OverflowTest is Test {
PuppyRaffle public raffle;
address public feeAddress = address(0xFEE);
​
function setUp() public {
raffle = new PuppyRaffle(
1 ether, // entranceFee
feeAddress,
10 // raffleDuration
);
}
​
function testTotalFeesOverflowBlocksWithdrawFees() public {
// Each round: 4 players * 1 ether = 4 ether collected
// fee = 4 ether * 20% = 0.8 ether added to totalFees per round
// uint64 max ~= 18.45 ether, so ~24 rounds triggers overflow
​
uint256 round;
while (raffle.totalFees() < 18 ether) {
round++;
for (uint256 i = 0; i < 4; i++) {
address player = address(uint160(round * 100 + i + 1));
vm.deal(player, 1 ether);
address[] memory entrants = new address[](1);
entrants[0] = player;
vm.prank(player);
raffle.enterRaffle{value: 1 ether}(entrants);
}
​
vm.warp(block.timestamp + 11);
raffle.selectWinner();
}
​
uint256 maxTotalFees = (uint256(1) << 64) - 1;
uint256 totalFeesBeforeOverflow = raffle.totalFees();
uint256 feeForNextRound = (4 * raffle.entranceFee() * 20) / 100;
uint256 expectedTotalFeesAfterOverflow =
(totalFeesBeforeOverflow + feeForNextRound) % (maxTotalFees + 1);
​
​
// Force one more round to trigger overflow
for (uint256 i = 0; i < 4; i++) {
address player = address(uint160(round * 100 + i + 100));
vm.deal(player, 1 ether);
address[] memory entrants = new address[](1);
entrants[0] = player;
vm.prank(player);
raffle.enterRaffle{value: 1 ether}(entrants);
}
vm.warp(block.timestamp + 11);
raffle.selectWinner();
​
console.logString("actual totalFees after overflow (wei):");
console.logUint(raffle.totalFees());
console.logString("Contract balance after overflow (wei):");
console.logUint(address(raffle).balance);
​
// withdrawFees now reverts because balance != totalFees
vm.expectRevert("PuppyRaffle: There are currently players active!");
raffle.withdrawFees();
}
}

Recommended Mitigation

Change totalFees from uint64 to uint256 and remove the unsafe cast.

- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
​
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour 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!