totalFees is a uint64 (max ~18.44 ETH) and the contract is compiled with Solidity
0.7.6, which has no built-in overflow checks. In selectWinner, fees are added as
totalFees = totalFees + uint64(fee), where fee is a uint256. Two problems:
The cast uint64(fee) silently truncates any fee larger than type(uint64).max.
The addition can silently overflow and wrap around.
Either way totalFees ends up far smaller than the fees actually held by the contract.
And the broken accounting then bricks withdrawals:
Because totalFees no longer matches the real ETH balance, this strict equality fails.
Likelihood: Medium — triggered whenever the collected fee for a round exceeds the
uint64 range (e.g. ~93+ players at a 1 ETH entrance fee), or accumulates past it over
several rounds. No attacker interaction is required; a popular raffle hits it.
Impact: High — fee accounting is corrupted: the protocol under-counts (loses) fees,
and withdrawFees can no longer satisfy its balance check, locking the fees in the
contract.
With 100 players the round fee is 20 ETH, which does not fit in uint64, so totalFees
is recorded as ~1.55 ETH instead of 20 ETH:
Use a uint256 for totalFees, remove the unsafe cast, and (optionally) compile with
Solidity >= 0.8 for built-in overflow checks:
Also avoid the strict equality in withdrawFees (use >=) so a forced ETH transfer
cannot brick withdrawals.
## 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.