Puppy Raffle

AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: high
Likelihood: low
Invalid

[M-05] Missing Zero Address Validation Allows Fee Loss

Root + Impact

Description

  • The changeFeeAddress() function does not validate that the new address is not address(0).

  • When the owner accidentally sets feeAddress to zero, all subsequent fee withdrawals will fail or send ETH to the burn address.

// Root cause in the codebase with @> marks to highlight the relevant section
function changeFeeAddress(address newFeeAddress) external onlyOwner {
@> feeAddress = newFeeAddress; // No zero address check
emit FeeAddressChanged(newFeeAddress);
}

Risk

Likelihood: Low

  • Reason 1 // Requires owner mistake

  • Reason 2 // Accidental zero address input

Impact: High

  • All accumulated fees sent to burn address

  • Fees permanently lost

  • No recovery mechanism

Proof of Concept

The following test demonstrates that the owner can set the fee address to zero without any validation, and subsequent fee withdrawals will send ETH to the burn address where it becomes permanently unrecoverable. This is a common mistake that can occur when interacting with contracts through scripts or frontends that don't properly validate inputs.

function testZeroFeeAddressLosesFees() public playersEntered {
// Owner accidentally sets zero address - no validation prevents this
puppyRaffle.changeFeeAddress(address(0));
assertEq(puppyRaffle.feeAddress(), address(0));
// Complete a raffle to accumulate fees
vm.warp(block.timestamp + duration + 1);
puppyRaffle.selectWinner();
// Get the fee amount that will be lost
uint256 feesAccumulated = puppyRaffle.totalFees();
assertGt(feesAccumulated, 0);
// Fees sent to zero address - permanently lost and unrecoverable
puppyRaffle.withdrawFees();
// The ETH is now burned at address(0), protocol has lost all revenue
assertEq(address(puppyRaffle).balance, 0);
}

Recommended Mitigation

Add zero address validation to prevent accidental misconfiguration. This is a standard security practice that should be applied to all address setters, especially those controlling fund flows. Consider also adding a two-step ownership transfer pattern for additional safety.

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

Lead Judging Commences

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