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

Integer Overflow in totalFees Variable Causes Incorrect Fee Tracking and Loss of Funds

wanzablock

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • The fee accumulation should:

    • Accurately track all fees collected across multiple raffles

    • Allow the protocol to withdraw accumulated fees at any time

    • Support protocols that run many raffles or have high entrance fees

  • Explain the specific issue or problem in one or more sentences

  • The totalFees variable uses uint64 for gas optimization (storage packing with feeAddress). However, this creates an overflow risk when fees exceed ~18.4 ETH. Since there's no overflow protection and Solidity 0.8.x only protects against arithmetic overflows in calculations (not type casting), the cast from uint256 to uint64 can silently overflow.

// Root cause in the codebase with @> marks to highlight the relevant section
// State variable declaration
address public feeAddress;
uint64 public totalFees = 0; // @> VULNERABILITY: uint64 can only hold ~18.4 ETH
​
function selectWinner() external {
// ... winner selection logic ...
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100; // uint256
// @> VULNERABILITY: Casting uint256 to uint64 can overflow
totalFees = totalFees + uint64(fee);
// ... rest of function ...
}

Risk

Likelihood:

Reason 1: A protocol running multiple raffles with moderate entrance fees (e.g., 1 ETH) would hit the overflow after just ~92 raffles with 4 players each (92 * 4 * 1 ETH * 20% = 73.6 ETH of fees, wraps around multiple times).

Reason 2: A single high-stakes raffle with expensive entrance fees (e.g., 10 ETH entrance, 100 players) would generate 200 ETH in fees in one raffle, immediately causing overflow.

Impact:

Impact 1: Accumulated fees are lost due to incorrect tracking. The protocol owner cannot withdraw the full amount of fees actually collected.

Impact 2: The withdrawFees() function checks require(address(this).balance == uint256(totalFees)), which will fail when totalFees has overflowed, making withdrawal impossible even for the correctly tracked portion.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.30;
​
import {Test} from "forge-std/Test.sol";
import {PuppyRaffle} from "./PuppyRaffle.sol";
​
contract IntegerOverflowTest is Test {
PuppyRaffle public puppyRaffle;
uint256 public entranceFee = 1 ether;
address public feeAddress = address(0x123);
function setUp() public {
puppyRaffle = new PuppyRaffle(
entranceFee,
feeAddress,
1 days
);
}
function testTotalFeesOverflow() public {
// Demonstrate overflow with realistic scenario
// Simulate 100 raffles with 4 players each at 1 ETH entrance
// Each raffle: 4 ETH collected, 0.8 ETH in fees (20%)
// Total fees after 100 raffles: 80 ETH
uint256 raffleCount = 100;
for (uint256 i = 0; i < raffleCount; i++) {
// Enter 4 players
address[] memory players = new address[](4);
for (uint256 j = 0; j < 4; j++) {
players[j] = address(uint160(i * 4 + j + 1));
}
// Fund and enter
vm.deal(address(this), 4 ether);
puppyRaffle.enterRaffle{value: 4 ether}(players);
// Wait for duration and select winner
vm.warp(block.timestamp + 1 days + 1);
puppyRaffle.selectWinner();
}
uint256 expectedFees = 80 ether; // 100 raffles * 0.8 ETH
uint64 actualTotalFees = puppyRaffle.totalFees();
// uint64 max is ~18.4 ETH, so 80 ETH wraps around multiple times
console.log("Expected fees (uint256):", expectedFees);
console.log("Actual totalFees (uint64):", actualTotalFees);
console.log("uint64 max:", type(uint64).max);
// The value will be completely wrong due to overflow
assertNotEq(expectedFees, uint256(actualTotalFees));
assertTrue(actualTotalFees < type(uint64).max);
}
function testSingleHighValueRaffleOverflow() public {
// Single high-stakes raffle
uint256 highEntranceFee = 10 ether;
PuppyRaffle highStakesRaffle = new PuppyRaffle(
highEntranceFee,
feeAddress,
1 days
);
// 50 players enter at 10 ETH each
address[] memory players = new address[](50);
for (uint256 i = 0; i < 50; i++) {
players[i] = address(uint160(i + 1));
}
// Total collected: 500 ETH
// Fees (20%): 100 ETH
vm.deal(address(this), 500 ether);
highStakesRaffle.enterRaffle{value: 500 ether}(players);
vm.warp(block.timestamp + 1 days + 1);
highStakesRaffle.selectWinner();
uint256 expectedFee = 100 ether;
uint64 actualFee = highStakesRaffle.totalFees();
console.log("Expected fee: 100 ETH");
console.log("Actual totalFees:", uint256(actualFee) / 1 ether, "ETH");
console.log("Overflow occurred!");
// uint64 max is ~18.4 ETH, so 100 ETH causes massive overflow
assertTrue(uint256(actualFee) < expectedFee);
// Withdrawal will fail because balance != totalFees
vm.expectRevert();
highStakesRaffle.withdrawFees();
}
function testExactOverflowBoundary() public {
// Find exact point where overflow occurs
uint64 maxUint64 = type(uint64).max;
console.log("uint64 max value:", maxUint64);
console.log("uint64 max in ETH:", maxUint64 / 1 ether);
// Setup raffle that will hit exactly max value
uint256 targetFee = uint256(maxUint64) + 1 ether;
// entranceFee * 20% * players = targetFee
// If entranceFee = 1 ether and 4 players per raffle
// Fee per raffle = 0.8 ether
// Number of raffles = targetFee / 0.8 ether
uint256 rafflesNeeded = (targetFee / 0.8 ether) + 1;
console.log("Raffles needed to overflow:", rafflesNeeded);
// Result: ~23 raffles with 4 players at 1 ETH entrance
// This proves overflow is easily achievable in normal operation
}
}

Recommended Mitigation

- remove this code
+ add this code// We do some storage packing to save gas
address public feeAddress;
- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
​
function selectWinner() external {
// ... winner selection ...
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
// ... rest of function ...
}
Updates

Lead Judging Commences

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