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

withdrawFees() sends ETH to arbitrary feeAddress with no access control validation

squashingbugs007

Root + Impact

Description

  • Normal behavior: PuppyRaffle collects an entrance fee from every participant. After a winner is selected, the protocol takes a 20% cut stored in totalFees. The owner calls withdrawFees() to send accumulated fees to feeAddress. The owner can also update feeAddress at any time via changeFeeAddress().

  • The issue: There is no validation, timelock, or access restriction on what address feeAddress can be set to. A malicious or compromised owner can call changeFeeAddress() to set feeAddress to any arbitrary address — including an attacker-controlled contract — immediately before calling withdrawFees(). All accumulated protocol fees are then sent to the attacker with no recourse.

  • This is a centralization risk combined with a missing input validation vulnerability. The feeAddress should either be immutable (set once at deployment) or protected by a timelock and multisig mechanism before any change takes effect.

// src/PuppyRaffle.sol#L157-L163
// @> feeAddress is set by owner with no restrictions
function withdrawFees() external onlyOwner {
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
// @> Low-level call to feeAddress — no validation of what feeAddress is
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
​
// @> Owner can change feeAddress to any address at any time
function changeFeeAddress(address newFeeAddress) external onlyOwner {
feeAddress = payable(newFeeAddress);
emit FeeAddressChanged(newFeeAddress);
}

Risk

Likelihood:

  • The owner can call changeFeeAddress() at any time before calling withdrawFees() — no conditions required.

  • No timelock, multisig, or governance mechanism exists to delay or reject a malicious address change.

  • A compromised owner private key is sufficient to execute the full attack in two transactions.

Impact:

  • All accumulated protocol fees (totalFees) can be redirected to an attacker-controlled address in a single transaction.

  • Fees belonging to the protocol are permanently lost with no recovery mechanism.

  • Player trust in the protocol is destroyed — the raffle fee collection is completely insecure

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
​
contract AttackerWallet {
address owner;
constructor() { owner = msg.sender; }
receive() external payable {}
function drain() external {
payable(owner).transfer(address(this).balance);
}
}
​
// Attack sequence (executed by compromised owner):
// Step 1: Deploy attacker wallet
AttackerWallet wallet = new AttackerWallet();
​
// Step 2: Redirect fee address to attacker wallet
// (Can be done immediately before withdrawal — no delay)
puppyRaffle.changeFeeAddress(address(wallet));
​
// Step 3: Withdraw fees — all go to attacker wallet
puppyRaffle.withdrawFees();
​
// Step 4: Drain attacker wallet to personal EOA
wallet.drain();
​
// Result: All feesToWithdraw drained from protocol
// Honest players and protocol have no recourse
​
function testArbitrarySendEth() public {
// Enter raffle with 4 players to accumulate fees
address[] memory players = new address[](4);
for (uint i = 0; i < 4; i++) players[i] = address(uint160(i+1));
puppyRaffle.enterRaffle{value: 4 ether}(players);
​
// Fast-forward to end of raffle and select winner
vm.warp(block.timestamp + duration + 1);
puppyRaffle.selectWinner();
​
// Owner changes fee address to attacker
vm.prank(owner);
puppyRaffle.changeFeeAddress(attacker);
​
// Owner withdraws — fees go to attacker
vm.prank(owner);
puppyRaffle.withdrawFees();
​
// Assert attacker received fees
assert(attacker.balance > 0);
}

Recommended Mitigation

- address payable public feeAddress;
+ address payable public immutable feeAddress;
​
constructor(
uint256 _entranceFee,
address _feeAddress,
uint256 _raffleDuration,
string memory _commonImageUri
) ERC721("Puppy Raffle", "PR") {
entranceFee = _entranceFee;
+ require(_feeAddress != address(0), "PuppyRaffle: fee address cannot be zero");
feeAddress = payable(_feeAddress);
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
previousWinner = _feeAddress;
commonImageUri = _commonImageUri;
}
​
- function changeFeeAddress(address newFeeAddress) external onlyOwner {
- feeAddress = payable(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!