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

Unsafe `uint64` cast in `PuppyRaffle::selectWinner` truncates `totalFees` and permanently locks the protocol's fees

furkanumut

Root + Impact

Description

Normally, accrued fees should be tracked in a type wide enough to hold any realistic value, so that the recorded fees always match the ETH actually retained for the fee address. However, the current implementation narrows the computed fee to 64 bits, corrupting the accounting the moment a single round's fee exceeds ~18.45 ETH.

The vulnerability exists in PuppyRaffle.sol#L134 (and PuppyRaffle.sol#L158).

Root Cause: Integer truncation — silent narrowing cast uint256 → uint64.

Vulnerable Code:

// File: src/PuppyRaffle.sol — Lines 29–30, 131–134, 157–158
​
address public feeAddress;
@> uint64 public totalFees = 0; // BUG: far narrower than the uint256 fee accumulated into it
​
// selectWinner():
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
@> totalFees = totalFees + uint64(fee); // BUG: truncates modulo 2^64 when fee >= ~18.45 ETH
​
// withdrawFees():
@> require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

Why This Is Exploitable:

In Solidity, an explicit narrowing cast truncates silently — it does not revert. A 100-player round at a 1 ETH ticket produces a 20 ETH fee; uint64(20e18) records only 1553255926290448384 wei (~1.55 ETH). The contract, however, still retains the full 20 ETH. Because withdrawFees requires the balance to equal totalFees exactly, and no function can skim the surplus, the equality can never hold again: every subsequent withdrawFees call reverts, freezing the entire fee balance (not just the truncated delta) forever.

Risk

Likelihood:

  • Triggers deterministically once a single round's 20% fee reaches 2^64 wei — a ~92.2 ETH pot at a 1 ETH ticket, or a smaller pot at a higher ticket price.

  • No attacker action is required; normal protocol growth into a popular raffle reaches the threshold.

Impact:

  • Serious accounting corruption: ~18.45 ETH of fee accounting vanishes silently in the triggering round.

  • Permanent freezing of the protocol's entire fee revenue from that point on (all rounds' fees, including the untruncated remainder).

Proof of Concept

function testPoC_TotalFeesUint64TruncationLocksFees() public {
uint256 n = 100;
address[] memory players = new address[](n);
for (uint256 i = 0; i < n; i++) {
players[i] = address(uint160(i + 1000));
}
puppyRaffle.enterRaffle{value: entranceFee * n}(players);
​
vm.warp(block.timestamp + duration + 1);
puppyRaffle.selectWinner();
​
uint256 expectedFee = (entranceFee * n * 20) / 100; // 20 ether
uint256 recordedFee = uint256(puppyRaffle.totalFees());
console2.log("expected 20% fee (wei):", expectedFee);
console2.log("recorded totalFees after uint64 cast (wei):", recordedFee);
​
assertTrue(recordedFee < expectedFee);
assertEq(recordedFee, expectedFee - 2 ** 64); // silent truncation modulo 2^64
​
// recorded fees no longer match the real balance -> withdrawal bricked forever
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
}

Run: forge test --match-test testPoC_TotalFeesUint64TruncationLocksFees -vv

Output (actual run):

[PASS] testPoC_TotalFeesUint64TruncationLocksFees() (gas: 5419650)
Logs:
expected 20% fee (wei): 20000000000000000000
recorded totalFees after uint64 cast (wei): 1553255926290448384

  1. 100 unique players enter a raffle with a 1 ETH ticket (100 ETH pot). Duration elapses.

  2. Anyone calls selectWinner. The computed fee is 20 ETH, but uint64(fee) stores 20e18 mod 2^64 = 1553255926290448384.

  3. totalFees() returns ~1.55 ETH while the contract holds 20 ETH of fees; withdrawFees() reverts with "There are currently players active!" and can never succeed again.

Recommended Mitigation

Track fees in a full-width uint256 and remove the narrowing cast, so recorded accounting can never diverge from the retained ETH:

Proposed Fix:

// File: src/PuppyRaffle.sol
​
- uint64 public totalFees = 0;
+ uint256 public totalFees = 0;
// ...
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees + fee;

References:

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 6 hours 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!