Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

[M-1] No Zero-Address Validation in changeFeeAddress

0xaljzoli

[M-1] No Zero-Address Validation in changeFeeAddress

Title: No Zero-Address Validation in changeFeeAddress

Impact: Medium

Likelihood: Medium

Description

  • The changeFeeAddress function allows the contract owner to update the destination address where protocol fees are sent. This is an administrative function intended to let the owner redirect fee payments to a different wallet as needed.

  • The function does not validate that the new address is not address(0). If the owner accidentally or maliciously sets the fee address to the zero address, all accumulated protocol fees will be permanently burned when withdrawFees or selectWinner attempts to send ETH to that address. The zero address is a common human error vector in Ethereum development, which is why OpenZeppelin and most audited contracts explicitly guard against it.

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

Risk

Likelihood:

  • This occurs when the contract owner calls changeFeeAddress with an unintentionally empty or zero address. Since the function takes an address parameter, passing an uninitialized variable or a literal address(0) will silently succeed without any revert or warning.

  • Multi-sig wallets or governance contracts could also pass a zero address if there is a bug in the proposal creation process. The lack of validation means the error propagates silently until fees are actually withdrawn, at which point the damage is irreversible.

Impact:

  • When selectWinner runs, 20% of the total collected ETH is sent to feeAddress. If feeAddress is address(0), this ETH is permanently burned and cannot be recovered by anyone, including the owner.

  • The withdrawFees function calls feeAddress.call{value: feesToWithdraw}(""). Sending ETH to address(0) succeeds on Ethereum (the call returns true), meaning no revert occurs and the fee withdrawal appears to succeed. However, the ETH is irrecoverably lost. Over multiple raffle rounds, this can accumulate to a significant amount of burned protocol revenue.

Proof of Concept

function testFeeAddressSetToZero() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
// Owner accidentally sets feeAddress to address(0)
vm.prank(owner);
puppyRaffle.changeFeeAddress(address(0));
​
// selectWinner mints NFT and sends 80% to winner
// 20% fee is sent to address(0) — permanently burned
uint256 expectedFee = ((entranceFee * 4) * 20) / 100;
uint256 contractBalanceBefore = address(puppyRaffle).balance;
​
puppyRaffle.selectWinner();
​
// The fee was "successfully" sent to address(0)
// but the ETH is gone forever
uint256 contractBalanceAfter = address(puppyRaffle).balance;
​
// Fee is recorded in totalFees but cannot be recovered
assertEq(puppyRaffle.totalFees(), expectedFee);
// Owner tries to withdraw — but fees already went to address(0)
// (if selectWinner sent them), or withdrawFees sends them to address(0)
}

This PoC demonstrates that when the owner sets feeAddress to address(0), the fee distribution in selectWinner or withdrawFees sends ETH to the zero address without reverting. On Ethereum, sending ETH to address(0) via a .call does not revert — it succeeds and the ETH is permanently lost. The owner has no way to recover these funds once they are sent, and the error is completely silent since no require statement catches it.

Recommended Mitigation

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

Adding a simple require check ensures that the fee address can never be set to address(0). This is a standard defensive programming practice recommended by OpenZeppelin's Ownable pattern and virtually all major audit firms. The revert message makes it immediately clear to the caller what went wrong, preventing silent misconfiguration. This single-line fix eliminates the entire class of zero-address fee loss bugs.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!