`totalFees` truncation via unsafe `uint64` cast permanently locks fees
In selectWinner at L161, the fee amount is truncated from uint256 to uint64 before accumulation:
uint64 has a maximum value of approximately 18.44 ETH (2^64 - 1 wei). If a single round collects more than ~92.2 ETH total (making fee = 20% exceed 18.44 ETH), the uint64(fee) cast silently discards the upper bits.
For example, with 100 players at 1 ETH entrance fee: fee = 20 ETH, but uint64(20 * 10^18) wraps to approximately 1.55 ETH. The remaining 18.45 ETH stays in the contract but is untracked by totalFees.
This directly breaks withdrawFees (L189), which requires address(this).balance == uint256(totalFees). After truncation, balance > totalFees, and the strict equality check fails permanently. The fees can never be withdrawn.
Additionally, repeated uint64 additions across multiple rounds can silently overflow, wrapping totalFees back toward zero.
Exploit Scenario
A raffle round with 100 players at 1 ETH entrance fee concludes.
fee= 20 ETH.uint64(20 ETH)truncates to ~1.55 ETH.totalFeesrecords 1.55 ETH instead of 20 ETH.After
selectWinnersends the 80% prize pool, the contract holds ~20 ETH in fees.withdrawFeeschecksaddress(this).balance == uint256(totalFees), i.e.,20 ETH == 1.55 ETH. This fails.The 20 ETH in fees is permanently locked.
Recommendations
Short term: Change totalFees from uint64 to uint256 to match the type of fee. Remove the truncating cast:
Long term: On Solidity 0.7.6, use SafeMath for all arithmetic to catch overflow at runtime. Alternatively, upgrade to Solidity 0.8+ where overflow reverts by default.
Lead Judging Commences
[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.
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.