selectWinner computes the round's fee as a uint256 and then downcasts it to uint64 when adding it to totalFees:
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.
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.
Expected: totalFees increases by the full fee. Actual: it increases by fee mod 2^64, losing the overflowed portion.
Store fees in uint256 and drop the narrowing cast entirely:
Widening totalFees to uint256 removes this cast-truncation loss (and the related accumulator-overflow issue), and keeps the fee accounting exact.
## 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.
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsThe contest is complete and the rewards are being distributed.