Submission Details
Impact:
Likelihood:
Missing access control on withdrawFees allows anyone to trigger fee withdrawal
# Missing access control on `PuppyRaffle::withdrawFees` allows anyone to trigger fee withdrawal
## Description
The `withdrawFees` function sends accumulated protocol fees to the `feeAddress` but has no access control modifier. Any external account or contract can call this function at any time, as long as the balance invariant check passes.
While the funds are always sent to `feeAddress` (not the caller), the lack of access control means any third party can force a withdrawal at a time that may not be desired by the protocol owner -- for example, triggering a withdrawal before enough fees have accumulated, or front-running an owner's call.
```solidity
@> function withdrawFees() external {
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
```
## Risk
**Likelihood**:
* The function is `external` with no access control, so any EOA or contract can call it at any time
* The only requirement is that `address(this).balance == uint256(totalFees)`, which holds true whenever there are no active raffle players
**Impact**:
* Anyone can trigger fee withdrawals at arbitrary times, removing the protocol owner's control over treasury management
* An attacker can front-run the owner's withdrawal transaction for griefing purposes
## Proof of Concept
1. Four players enter the raffle and the raffle duration elapses. `selectWinner` is called, distributing the prize and recording 20% as `totalFees`.
2. At this point, any random address -- not the owner, not the `feeAddress` -- can call `withdrawFees` and it succeeds, because there is no `onlyOwner` or similar modifier.
3. The test demonstrates that a completely unrelated address (`address(0xdead)`) can trigger the fee withdrawal to `feeAddress`.
```solidity
function testAnyoneCanWithdrawFees() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
puppyRaffle.selectWinner();
uint256 expectedFees = (entranceFee * 4 * 20) / 100;
// A random unrelated address calls withdrawFees
vm.prank(address(0xdead));
puppyRaffle.withdrawFees();
// Fees were successfully sent to feeAddress, triggered by a stranger
assertEq(address(feeAddress).balance, expectedFees);
assertEq(puppyRaffle.totalFees(), 0);
}
```
## Recommended Mitigation
Restrict `withdrawFees` to the contract owner:
```diff
- function withdrawFees() external {
+ function withdrawFees() external onlyOwner {
require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
```
Rewards breakdown
This contest has a flat reward structure with the following payouts:
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.