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

`changeFeeAddress()` Missing Zero-Address Validation — Owner Can Permanently Redirect Fees to Null Address

h3xu

changeFeeAddress() Missing Zero-Address Validation — Owner Can Permanently Redirect Fees to Null Address

Description

PuppyRaffle.changeFeeAddress() allows the contract owner to update the address that receives accumulated protocol fees. The function lacks a zero-address guard: if the owner passes address(0) as newFeeAddress (intentionally or by mistake), feeAddress is set to address(0) and all future fee withdrawals via withdrawFees() will silently burn ETH to the zero address.

While this is an owner-only action, the missing guard creates risk of catastrophic fund loss from owner error, a compromised owner key, or a malicious ownership transfer. The severity is amplified because changeFeeAddress emits an event (FeeAddressChanged) that could be used to verify the change — but there is no on-chain enforcement.

function changeFeeAddress(address newFeeAddress) external onlyOwner {
@> // No require(newFeeAddress != address(0)) guard
feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress);
}

Risk

Likelihood: Low

  • This requires the contract owner to call changeFeeAddress(address(0)) — an action only the privileged owner can make.

  • While human error is possible, accidental zero-address submission is uncommon in practice with wallet UX safeguards.

  • No unprivileged party can trigger this without a compromised owner key.

Impact: Medium

  • If triggered, all accumulated fees in the contract become unrecoverable (sent to address(0) on next withdrawFees() call).

  • The loss is proportional to fees accumulated before anyone notices — could range from zero to all lifetime protocol fees.

Severity: Low

Proof of Concept

The following sequence demonstrates the attack through an accidental or malicious owner call:

  1. Owner calls changeFeeAddress(address(0)) — accepted without revert.

  2. Later, withdrawFees() succeeds because the balance invariant is satisfied.

  3. (bool success,) = feeAddress.call{value: feesToWithdraw}("") where feeAddress == address(0).

  4. All accumulated fees are burned to address(0) — success = true, no revert.

// feeAddress = address(0) set by owner (accidentally or maliciously)
puppyRaffle.changeFeeAddress(address(0));
​
// Later, someone triggers withdrawFees()
// totalFees = 5 ETH, address(this).balance == 5 ETH → passes guard
// (bool success,) = address(0).call{value: 5 ETH}(""); → success=true
// 5 ETH burned

Recommended Mitigation

Add a zero-address check at the beginning of changeFeeAddress():

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

Additionally, consider adding a two-step ownership / address-change mechanism (propose + accept) to eliminate single-transaction errors for high-value parameter updates.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours 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!