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.
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 {
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();
puppyRaffle.changeFeeAddress(address(0));
assertEq(puppyRaffle.feeAddress(), address(0));
uint256 feesBefore = uint256(puppyRaffle.totalFees());
emit log_named_uint("Total fees to withdraw", feesBefore);
puppyRaffle.withdrawFees();
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