Title: Typecasting from uint256 to uint64 in selectWinner() leads to overflow and incorrect fee calculation
Impact: High — Fee accounting breaks silently, wrong amounts withdrawn.
Likelihood: High — Triggers after ~18 rounds with 1 ETH entrance fee.
Reference Files: src/PuppyRaffle.sol:30,134
Description:
totalFees is declared as uint64 but accumulates uint256 values via an unsafe downcast. In Solidity 0.7.6, this silently truncates and wraps without reverting. The vulnerable code:
After enough rounds, the accumulated fees exceed type(uint64).max and wrap to a small value.
Impact: High. withdrawFees() sends the wrong amount because totalFees has wrapped. All fee accounting is corrupted.
Likelihood: High. With 1 ETH entrance fee, overflow occurs after approximately 18 raffle rounds. Inevitable over time.
A contract running 20 raffle rounds will have totalFees wrap to a value far below the actual accumulated fees.
The accumulator silently wraps — type(uint64).max + 0.8 ether becomes approximately 0.3 ether.
Change totalFees to uint256 and remove the typecast:
Matching types prevent silent truncation and wrapping.
## 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.