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

Missing Zero-Address Check in changeFeeAddress() Can Permanently Lock All Fees

slientj

Root + Impact

Description

Normal Behavior:
The changeFeeAddress function allows the owner to update feeAddress, which receives all accumulated fees when withdrawFees is called.

Specific Issue:
Neither changeFeeAddress nor the constructor validates that the new feeAddress is not address(0). If feeAddress is set to address(0), all future withdrawFees calls will send ETH to the zero address, permanently losing the funds.

// @> No zero-address check
function changeFeeAddress(address newFeeAddress) external onlyOwner {
feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress);
}

Risk

Likelihood:

  • The owner can accidentally set feeAddress to address(0).

  • If ownership is transferred or compromised, an attacker could deliberately set it to address(0).

Impact:

  • All accumulated fees are permanently lost when withdrawFees is called.

  • The contract's fee mechanism becomes unusable.

Proof of Concept

The test first sets feeAddress to address(0) via changeFeeAddress. It then calls withdrawFees, which sends the entire balance to address(0). The ETH is permanently lost because the zero address cannot spend or return it. The assertion confirms the contract balance drops to 0.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract ZeroAddressFeeTest is Test {
PuppyRaffle public raffle;
​
function setUp() public {
raffle = new PuppyRaffle(
address(this), // feeAddress initially set to this contract
1 ether,
10
);
​
// A player enters to accumulate fees
address player = address(0x1);
vm.deal(player, 1 ether);
vm.prank(player);
raffle.enterRaffle{value: 1 ether}();
}
​
function testZeroAddressLocksFees() public {
// 1. Owner sets feeAddress to address(0)
raffle.changeFeeAddress(address(0));
​
// 2. Confirm feeAddress is now zero
assertEq(raffle.feeAddress(), address(0));
​
// 3. Record balance before withdrawFees
uint256 balanceBefore = address(raffle).balance;
console.log("Contract balance before withdrawFees:", balanceBefore);
​
// 4. withdrawFees sends ETH to address(0), funds are lost
raffle.withdrawFees();
​
uint256 balanceAfter = address(raffle).balance;
console.log("Contract balance after withdrawFees:", balanceAfter);
​
// 5. Assert that the contract balance is now 0 and the ETH is gone
assertEq(balanceAfter, 0);
assertEq(address(0).balance, 0); // Zero address cannot receive funds
}
}

Recommended Mitigation

Also add the same check in the constructor when _feeAddress is assigned.

function changeFeeAddress(address newFeeAddress) external onlyOwner {
+ require(newFeeAddress != address(0), "PuppyRaffle: Fee address cannot be zero");
feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress);
}
Updates

Lead Judging Commences

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

[H-01] Potential Loss of Funds During Prize Pool Distribution

## Description In the `selectWinner` function, when a player has refunded and their address is replaced with address(0), the prize money may be sent to address(0), resulting in fund loss. ## Vulnerability Details In the `refund` function if a user wants to refund his money then he will be given his money back and his address in the array will be replaced with `address(0)`. So lets say `Alice` entered in the raffle and later decided to refund her money then her address in the `player` array will be replaced with `address(0)`. And lets consider that her index in the array is `7th` so currently there is `address(0)` at `7th index`, so when `selectWinner` function will be called there isn't any kind of check that this 7th index can't be the winner so if this `7th` index will be declared as winner then all the prize will be sent to him which will actually lost as it will be sent to `address(0)` ## Impact Loss of funds if they are sent to address(0), posing a financial risk. ## Recommendations Implement additional checks in the `selectWinner` function to ensure that prize money is not sent to `address(0)`

Support

FAQs

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

Give us feedback!