Scope: src/PuppyRaffle.sol
The totalFees variable is declared as uint64, which has a maximum value of ~18.4 ETH. When fees exceed this amount, the value silently overflows (Solidity 0.7.6 doesn't have overflow protection), causing fees to be lost.
Normal behavior: The protocol should accumulate 20% of all entrance fees for the feeAddress to withdraw.
The issue: Solidity 0.7.6 does not have built-in overflow protection. When totalFees exceeds type(uint64).max (18,446,744,073,709,551,615 wei ≈ 18.4 ETH), the value wraps around to a small number, losing accumulated fees.
Likelihood:
With 1 ETH entrance fee, 20% fee = 0.2 ETH per player
After ~92 players total (across rounds), fees exceed 18.4 ETH
Popular raffles will easily exceed this threshold
Impact:
Protocol loses collected fees permanently
feeAddress cannot withdraw proper amounts
Economic model of the protocol breaks down
Explanation: The test creates a raffle with 18 ETH entrance fee, runs two rounds with 4 players each. Total fees should be 28.8 ETH (72 ETH × 20% × 2), but after overflow, the recorded fees are much less than expected, proving fees are lost.
Explanation: Change totalFees from uint64 to uint256 to handle any realistic fee amount. Also consider upgrading to Solidity 0.8.x which has built-in overflow protection.
## 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.