Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Integer truncation in PuppyRaffle: selectWinner permanently locks all protocol fees

angmarwitchking

Root + Impact

selectWinner computes the protocol fee as a uint256 but stores it in a uint64 state variable through an unchecked downcast.

Description

When a single raffle’s fee exceeds type(uint64).max (~18.45 ETH), uint64 (fee) silently truncates (wraps modulo 2^64), so totalFees records far less than the fees actually collected. Because the contract is compiled with Solidity ^0.7.6 (no built-in overflow protection) and this line uses the native + rather than SafeMath, the accumulation is unchecked as well.


This causes corruption of the accounting that withdrawFees relies on. Once totalFees is undercounted, the contract's balance can never equal totalFees, so this require reverts on every call and withdrawFees becomes permanently uncallable. The collected fees are then locked in the contract with no recovery path.

// Unsafe downcast in selectWinner
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee); // totalFees is declared `uint64`
​
// The code that becomes corrupted in withdrawFees
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

Risk

Likelihood:

High. The truncation occurs whenever one raffle’s fee exceeds ~18.45 ETH — reached at ~93 players at a 1 ETH entrance fee, and there is no maximum player cap. No malicious actor is required; a sufficiently popular raffle triggers it through normal use.

Impact:

High. All protocol fees from the affected raffle are permanently frozen. At 93 players (1 ETH entry), 18.6 ETH becomes unwithdrawable, scaling with raffle size. The loss is irreversible — there is no alternate withdrawal path and totalFees cannot be corrected on-chain.

Proof of Concept

Add to the test suite (uses the standard setUp deploying puppyRaffle with a 1e18 entranceFee).

With 93 players, the contract holds 18.6 ETH in fees, but totalFees only records 0.153 ETH. The low 64 bits of the real value (18.6 - 2^64 = 153255926290448384). withdrawFees then reverts, stranding the 18.6 of ETH.

function test_totalFeesOverflowLocksWithdrawals() public {
uint256 numPlayers = 93; // fee exceeds uint64 max (~18.45 ETH)
address[] memory players = new address[](numPlayers);
for (uint256 i = 0; i < numPlayers; i++) {
players[i] = address(uint160(i + 1000)); // +1000 avoids the precompile range
}
puppyRaffle.enterRaffle{value: entranceFee * numPlayers}(players);
​
vm.warp(block.timestamp + duration + 1);
puppyRaffle.selectWinner();
​
uint256 recordedFees = puppyRaffle.totalFees();
uint256 actualFees = address(puppyRaffle).balance;
emit log_named_uint("recorded totalFees", recordedFees); // 153255926290448384 (~0.15 ETH)
emit log_named_uint("actual fee balance", actualFees); // 18600000000000000000 (18.6 ETH)
​
assertLt(recordedFees, actualFees); // accounting corrupted by truncation
​
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees(); // fees permanently locked
}
​
​
Overflow Output:
forge test --match-test test_totalFeesOverflowLocksWithdrawals -vvvv
​
Ran 1 test for test/Overflow.t.sol:PuppyRaffleReentrancyTest
[PASS] test_totalFeesOverflowLocksWithdrawals() (gas: 21347570)
Logs:
recorded totalFees: 153255926290448384
actual fee balance: 18600000000000000000
​

Recommended Mitigation

Store totalFees as a uint256 and remove unsafe downcast. A uin256 cannot overflow at any realistic fee volume and removing the cast eliminates the truncation entirely. Upgrading the compiler to >=0.8 would additionally add overflow protection to the accumulation.

- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
...
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-05] Typecasting from uint256 to uint64 in PuppyRaffle.selectWinner() May Lead to Overflow and Incorrect Fee Calculation

## 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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!