Puppy Raffle

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

Unsafe downcast uint64(fee) truncates when a single round's fee exceeds type(uint64).max, silently losing fees

Description

selectWinner computes the round's fee as a uint256 and then downcasts it to uint64 when adding it to totalFees:

uint256 totalAmountCollected = players.length * entranceFee;
uint256 fee = (totalAmountCollected * 20) / 100; // uint256
totalFees = totalFees + uint64(fee); // explicit narrowing cast

The cast uint64(fee) keeps only the low 64 bits of fee. If a single round's fee exceeds type(uint64).max (18_446_744_073_709_551_615 wei ≈ 18.446 ETH), the cast silently discards the high bits, so the amount actually recorded in totalFees is fee mod 2^64 — far less than the fee that was really collected in ETH.

fee is 20% of players.length * entranceFee, so a large entranceFee and/or a large field of players makes a >18.44 ETH single-round fee reachable. When it happens, the protocol under-counts its own fees: the ETH is in the contract, but totalFees reflects only the truncated remainder. The gap is unrecoverable, and it also desynchronises totalFees from address(this).balance, contributing to withdrawFees being unable to satisfy its equality check.

This is distinct from the uint64 accumulator overflow (which wraps when the running sum crosses the ceiling): here the loss happens in a single round whose fee alone overflows 64 bits during the narrowing cast, even starting from totalFees == 0.

Risk

Impact: Medium. Fees collected in a large round are silently truncated, permanently losing the high bits of that round's fee and corrupting fee accounting. Real ETH is stranded.

Likelihood: Low. Requires a single round's fee to exceed ~18.44 ETH, which needs a high entranceFee or a very large player set; plausible for a popular raffle but not the common case.

Proof of Concept

type(uint64).max = 18_446_744_073_709_551_615 wei (~18.446 ETH)
# Suppose a round collects enough that fee = 20 ETH = 20e18 wei
fee (uint256) = 20_000_000_000_000_000_000
uint64(fee) = 20_000_000_000_000_000_000 mod 2^64
= 1_553_255_926_290_448_384 # ~1.553 ETH recorded
# ~18.45 ETH of fee ETH sits in the contract, but totalFees rose by only ~1.55 ETH.

Expected: totalFees increases by the full fee. Actual: it increases by fee mod 2^64, losing the overflowed portion.

Recommended Mitigation

Store fees in uint256 and drop the narrowing cast entirely:

uint256 public totalFees = 0;
...
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + fee; // no downcast, no truncation

Widening totalFees to uint256 removes this cast-truncation loss (and the related accumulator-overflow issue), and keeps the fee accounting exact.

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!