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

Integer Overflow in totalFees Causes Loss of Protocol Fees

vikramgoutham289

Root + Impact

Root Cause: totalFees is declared as uint64 (max ~18.4 ETH) in Solidity 0.7.6 which lacks overflow protection, causing silent wraparound when fees exceed this limit.

Impact: Protocol permanently loses track of accumulated fees, owner cannot withdraw correct amounts, and significant revenue is lost to overflow.

Description

Normal Behavior: The totalFees variable should accurately track all fees collected from raffles so the owner can withdraw them later.

Issue: Solidity 0.7.6 does not have built-in overflow protection, and totalFees is declared as uint64 which has a maximum value of ~18.4 ETH. When fees exceed this value, the variable overflows and wraps around to a small number, causing permanent loss of fees.
// @> uint64 can only hold up to 18,446,744,073,709,551,615 wei (~18.4 ETH)
uint64 public totalFees = 0;
​
function selectWinner() external {
// ...
uint256 fee = (totalAmountCollected * 20) / 100;
// @> Unsafe cast and addition - will overflow!
totalFees = totalFees + uint64(fee);
// ...
}

Risk

Likelihood:HIGH

  • Reason 1 : With 1 ETH entrance fee and 100 players per raffle, overflow happens after ~92 raffles

  • Reason 2: Occurs whenever cumulative fees exceed ~18.4 ETH

Impact:

  • Impact 1:Owner cannot withdraw legitimate fees (they become locked or understated)

  • Impact 2: Protocol permanently loses track of collected fees

Proof of Concept

In Solidity versions before 0.8.0, arithmetic operations don't check for overflow/underflow. When a uint64 reaches its maximum value and you add more, it "wraps around" to start from 0 again.

function testIntegerOverflow() public {
uint64 maxUint64 = type(uint64).max; // 18,446,744,073,709,551,615
// Simulate near-max totalFees
uint256 entranceFee = 1 ether;
uint256 playersPerRound = 100;
uint256 feePerRound = (playersPerRound * entranceFee * 20) / 100; // 20 ETH fee per round
console.log("Fee per round:", feePerRound);
console.log("Max uint64:", maxUint64);
// After just 1 round with 100 players at 1 ETH each:
// Fee = 20 ETH = 20e18 wei
// But uint64 max is ~18.4e18
// OVERFLOW on first big raffle!
uint64 totalFees = 0;
uint64 fee = uint64(feePerRound); // This cast itself causes data loss!
console.log("Fee after unsafe cast:", fee);
// fee is now a corrupted small number, not 20 ETH
}

Recommended Mitigation

Use uint256: This type can hold up to ~1.15 × 10^77, which is practically unlimited for tracking ETH
Upgrade Solidity: Version 0.8.0+ has built-in overflow checks that revert on overflow
- pragma solidity ^0.7.6;
+ pragma solidity ^0.8.18;
​
// We do some storage packing to save gas
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 6 days 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!