Puppy Raffle

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

changeFeeAddress allows setting address(0), permanently locking all future fees.

Root + Impact

Description

The changeFeeAddress function (`https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L167`) has no zero-address validation:
https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L167
https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L168
https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L169
If the owner accidentally sets feeAddress to address(0), all future fee withdrawals when the withdrawFees function (`https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L157`) is called will send ETH to address(0):
https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L161
https://github.com/CodeHawks-Contests/ai-puppy-raffle/blob/08e5b1fc6939b8da7792b2d13e43000c519d8897/src/PuppyRaffle.sol#L162
This call succeeds (returns true) but the ETH is sent to the zero address and is permanently lost. There is no way to recover these funds.
// Root cause in the codebase with @> marks to highlight the relevant section

Risk

Likelihood:

Impact:

All accumulated protocol fees permanently lost if feeAddress is set to address(0)
Owner error (single transaction) can cause irreversible fund loss.

Proof of Concept

The following POC demostrates the effects of not validating protocol fee address before updating. Would result to loss of protocol fee

contract ZeroFeeAddressPoC is Test {
.....
function testZeroFeeAddressLocksFunds() public {
// Complete a raffle round to accumulate fees
for (uint256 i = 0; i < 4; i++) {
address player = address(uint160(i + 10));
vm.deal(player, entranceFee);
address[] memory newPlayers = new address[](1);
newPlayers[0] = player;
vm.prank(player);
puppyRaffle.enterRaffle{value: entranceFee}(newPlayers);
}
vm.warp(block.timestamp + 1 days + 1);
puppyRaffle.selectWinner();
// Owner accidentally sets feeAddress to address(0)
puppyRaffle.changeFeeAddress(address(0));
assertEq(puppyRaffle.feeAddress(), address(0));
// Now withdrawFees sends to address(0), losing the funds
uint256 feesBefore = uint256(puppyRaffle.totalFees());
emit log_named_uint("Total fees to withdraw", feesBefore);
// withdrawFees succeeds but funds go to address(0)
puppyRaffle.withdrawFees();
// feeAddress (address(0)) received the funds - they are lost forever
// No way to recover
assertEq(uint256(puppyRaffle.totalFees()), 0);
}
}

Recommended Mitigation

Add a zero-address check as thus:
feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress); - remove this code
require(newFeeAddress != address(0), "PuppyRaffle: Fee address cannot be zero");
feeAddress = newFeeAddress; + add this code
Updates

Lead Judging Commences

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