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

nezar_alhammadi

Root + Impact

Description

The PuppyRaffle contract tracks accumulated protocol fees using a uint64 state variable called totalFees. However, the fee amount is first computed as a uint256 and then unsafely truncated with an explicit cast uint64(fee) before being added. This silent truncation causes an integer overflow whenever the computed fee exceeds type(uint64).max (approximately 18.44 ETH), wrapping the value around to a much smaller number and permanently destroying the difference.

uint256 totalAmountCollected = players.length * entranceFee;
uint256 fee = (totalAmountCollected * 20) / 100;
​
// @> totalFees is uint64 but fee is uint256
// @> unsafe downcast silently discards the upper bits
totalFees = totalFees + uint64(fee); // @> OVERFLOW: high bits truncated

Because Solidity 0.7.x does not include built-in overflow protection for explicit casts, this truncation happens silently with no revert and no event emitted. The lost ETH remains locked in the contract but is no longer tracked by totalFees, making it permanently unrecoverable through withdrawFees().

Additionally, the withdrawFees() function enforces a strict balance check:

// @> if totalFees overflowed, this condition can never be satisfied
// @> because actual balance > uint256(totalFees)
require(address(this).balance == uint256(totalFees),
"PuppyRaffle: There are currently players active!");

This means that after an overflow event, withdrawFees() becomes permanently bricked — the owner can never withdraw any fees at all, even the small truncated amount that totalFees still tracks.

Risk

Likelihood:


  • The overflow threshold is type(uint64).max = 18,446,744,073,709,551,615 wei ≈ 18.44 ETH per raffle round

  • With an entranceFee of 1 ETH, only 93 players are needed to trigger overflow in a single round (93 * 1e18 * 20% = 18.6 ETH > 18.44 ETH)

  • With a lower entranceFee of 0.1 ETH, only 923 players are needed — a realistic number for a popular raffle

  • The overflow accumulates across rounds: even if each individual round does not overflow, totalFees is never reset to zero after withdrawFees() fails, so the cumulative sum will eventually overflow

  • This is a First Flight contest with intentional bugs, making it certain to be triggered

Impact

  • Protocol fees collected in ETH are permanently destroyed — they cannot be recovered by any function in the contract

  • The feeAddress wallet receives far less ETH than the protocol intends (20% of prize pool)

  • After overflow occurs, withdrawFees() is permanently bricked due to the strict balance equality check, meaning even correctly-tracked fees become unwithdrawable

  • The owner has no emergency mechanism to rescue the stuck ETH, resulting in total and irreversible loss of all accumulated fees

Proof of Concept

// Scenario: 100 players, entranceFee = 1 ETH
​
uint256 totalAmountCollected = 100 * 1e18; // 100 ETH in wei
uint256 fee = (totalAmountCollected * 20) / 100; // 20 ETH = 20_000_000_000_000_000_000
​
// type(uint64).max = 18_446_744_073_709_551_615 (~18.44 ETH)
// fee = 20_000_000_000_000_000_000 (~20.00 ETH)
​
// Truncation:
// 20_000_000_000_000_000_000 mod 2^64
// = 20_000_000_000_000_000_000 - 18_446_744_073_709_551_616
// = 1_553_255_926_290_448_384 (~1.55 ETH)
​
uint64(fee) == 1_553_255_926_290_448_384; // only ~1.55 ETH tracked
// ~18.45 ETH silently lost forever

Formal Verification Evidence (Certora Prover):

The Certora Prover rule totalFees_no_overflow was formally verified and returned VIOLATED, providing mathematical proof that totalFees can decrease after a call to selectWinner(). This is impossible under correct arithmetic and confirms that integer overflow is occurring:

Rule: totalFees\_no\_overflow Property: feesAfter >= feesBefore Result: VIOLATED Prover output: prover.certora.com/output/3088449/d25650413fc1466280b1d4664a294a41

The Prover found a concrete counterexample where calling selectWinner() causes totalFees to decrease — direct mathematical proof of the overflow.



Recommended Mitigation

The fix requires two changes: promoting totalFees from uint64 to uint256 to match the native size of fee calculations, and removing the unsafe downcast.

- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;

This eliminates the truncation entirely. Since fee is already a uint256, no cast is needed. The withdrawFees() balance check will also begin working correctly again because totalFees will accurately reflect the true amount of ETH owed to the fee recipient.
Additionally, consider upgrading to Solidity ^0.8.0 or using OpenZeppelin's SafeCast library to prevent similar silent truncation bugs elsewhere in the codebase.

Updates

Lead Judging Commences

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