Title: Dust ETH renders withdrawFees() function useless via strict balance equality
Impact: Medium — All fees permanently locked.
Likelihood: Low — Requires selfdestruct or accidental ETH transfer.
Reference Files: src/PuppyRaffle.sol:158
Description:
withdrawFees() requires the contract's ETH balance to exactly equal totalFees. Any dust ETH arriving via selfdestruct or accidental transfer breaks this equality permanently. The vulnerable code:
There is no recovery mechanism — once broken, fees can never be withdrawn.
Impact: Medium. All accumulated and future fees are permanently locked in the contract. No funds are stolen, but the protocol's fee mechanism is destroyed.
Likelihood: Low. Requires an attacker to deploy a selfdestruct contract targeting the PuppyRaffle address, or an accidental direct ETH transfer.
A single 1-wei selfdestruct is sufficient to permanently disable all fee withdrawals.
After this attack, address(puppyRaffle).balance exceeds totalFees by 1 wei, and the strict equality check fails permanently.
Use greater-than-or-equal since fees are tracked independently:
The contract tracks totalFees as a separate accounting variable — the balance check only needs to confirm sufficient funds exist, not an exact match.
## 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"); } ```
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.