The contract stores accrued fees in uint64, even though ETH balances and the local fee calculation use uint256. Solidity 0.7.6 silently truncates the explicit cast when the fee exceeds type(uint64).max. withdrawFees() then requires the real balance to equal the truncated value, making the legitimate fees permanently unavailable through the normal withdrawal path.
Likelihood: Medium
The trigger requires one fee or accumulated fees above 18.446744073709551615 ether. With the reference 1 ETH entry price, that means at least 93 entries in one round or multiple rounds without an intervening withdrawal. The constructor nevertheless accepts arbitrary entry fees, so this state is valid and reachable.
Impact: Medium
The full fee balance remains in the contract but cannot be withdrawn to the configured feeAddress. The issue does not redirect the winner prize or grant an attacker ownership; it causes loss of availability for accrued protocol fees.
Observed against challenge commit 08e5b1fc6939b8da7792b2d13e43000c519d8897:
Store totalFees as uint256 and remove the narrowing cast. Because the project compiles with Solidity 0.7.6, use SafeMath for additions or upgrade to Solidity 0.8+ after checking compatibility.
Regression tests should cover the exact uint64 boundary, values immediately above it, multiple-round accumulation, and a successful withdrawal of the entire tracked fee balance.
## 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.