totalFees should hold all fees owed to the fee address.
It is a uint64 (max ~18.45 ETH) and the contract uses solc 0.7.6, which has no overflow checks. uint64(fee) truncates and totalFees + uint64(fee) wraps silently. Once it wraps, totalFees is smaller than the real fee balance, and withdrawFees (which requires address(this).balance == totalFees) reverts forever.
Likelihood:
A single raffle with 93 players at 1 ETH (or a few smaller raffles over time) passes the uint64 limit; fees accumulate across rounds, so it happens naturally in normal use.
Impact:
All fees are locked in the contract permanently; the owner can never withdraw them. In the PoC 18.6 ETH of fees is recorded as ~0.153 ETH and withdrawFees reverts.
All tests run in a test contract that deploys PuppyRaffle(1e18, address(99), 1 days) and uses the helpers _players(start,n) (distinct addresses) and _enter(p) (pays 1e18 * p.length). Full file: test/AuditPoC.t.sol, forge test --mc AuditPoC passes.
Use uint256 for totalFees and remove the cast, and use solc >= 0.8 (or SafeMath):
## 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.