PuppyRaffle accumulates a 20% protocol fee into totalFees each time a winner is selected, which is later withdrawn by the fee address.
The per-round fee (a uint256) is cast to uint64 before being added to totalFees, and uint64 overflows silently at approximately 18.4 ETH, wrapping totalFees to a near-zero value and permanently trapping all fees above that threshold in the contract.
Likelihood:
A raffle with a large player count or high entrance fee can accumulate more than 18.4 ETH in protocol fees over multiple rounds; the risk increases as the protocol grows.
Impact:
All ETH accumulated beyond the overflow threshold is permanently locked in the contract; the owner can never withdraw it, and there is no recovery path.
Suppose 100 players each pay 1 ETH to enter. The 20% fee for that round is 20 ETH. uint64(20 ether) overflows: 20e18 mod 2^64 ≈ 1.55 ETH, so totalFees is set to ~1.55 ETH while ~18.45 ETH is silently discarded.
The PoC confirms that a single large raffle round can cause totalFees to wrap, permanently locking the overflow amount.
Store totalFees as uint256 throughout, eliminating the narrowing cast entirely.
## 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.