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

Missing Zero-Address Check for feeAddress

leoyeesir

Root + Impact

Description

The changeFeeAddress function (line 195) allows the contract owner to update the address that receives protocol fees. However, the function lacks a validation check to ensure the newFeeAddress is not address(0).

In Solidity version 0.7.6, performing a .call{value: ...}("") to address(0) does not revert; it returns success = true. Consequently, if the admin accidentally sets the fee address to zero, the withdrawFees function will execute successfully, but all accumulated protocol fees will be sent to the burn address, resulting in the permanent loss of those funds.

function changeFeeAddress(address newFeeAddress) external onlyOwner {
feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress);
}

Risk

Likelihood:

Low. This vulnerability requires a manual administrative error (the owner providing the wrong input). However, since there is no way to "un-burn" funds once they are sent to address(0), the lack of a safety check is a significant oversight.

Impact:

High. The impact is the permanent destruction of protocol revenue. Once withdrawFees is called with a zero-address set:

  • The totalFees counter is reset to 0.

  • The ETH is transferred to address(0).

  • The protocol owner loses 100% of the accumulated fees for that period with no possibility of recovery.

Proof of Concept

The test demonstrates that the owner can successfully set the feeAddress to address(0). When the raffle concludes and fees are withdrawn, the transaction succeeds, the contract's totalFees state is cleared, but the ETH is sent to the zero address instead of a wallet controlled by the protocol.

function testZeroAddressFeeCheck() public {
// 1. Owner accidentally sets fee address to address(0)
vm.prank(puppyRaffle.owner());
puppyRaffle.changeFeeAddress(address(0));
​
// 2. Conduct a raffle to generate fees
_enterFourPlayers();
vm.warp(block.timestamp + duration + 1);
puppyRaffle.selectWinner();
​
uint64 feesToWithdraw = puppyRaffle.totalFees();
assertGt(uint256(feesToWithdraw), 0);
​
// 3. Withdraw fees - the call succeeds despite sending to address(0)
puppyRaffle.withdrawFees();
​
// 4. Verification: ETH is lost to address(0) and internal state is reset
assertEq(address(0).balance, uint256(feesToWithdraw));
assertEq(uint256(puppyRaffle.totalFees()), 0);
console.log("Fees were successfully sent to address(0) and are permanently lost");
}
​

Recommended Mitigation

Add a require statement in the changeFeeAddress function to prevent setting the address to address(0). Additionally, consider emitting an event for better off-chain tracking.

function changeFeeAddress(address newFeeAddress) external onlyOwner {
// Add zero-address validation
require(newFeeAddress != address(0), "PuppyRaffle: Fee address cannot be zero");
feeAddress = newFeeAddress;
// Recommended: Emit an event
emit FeeAddressChanged(newFeeAddress);
}
​
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 10 days 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!