First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

withdrawFees breaks if totalFees becomes inaccurate

equious

Summary

Due to this function's accounting referencing the contract balance directly, any unaccounted for changes to the contract balance, combined with the functions requirement for the balance to equal totalFees, means these funds can never be withdrawn.

Vulnerability Details

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");
}

This function requires the contract's balance to equal the totalFees variable, updated when a winner is selected. If the contract's balance were to change and not be accounted for in totalFees, this function could not be callable.

This is achievable via a selfDestruct(address(puppyRaffle)) function on another contract. The balance of the destructed contract would be dumped onto Puppy Raffle, breaking the withdraw function.

Impact

Funds would become irretrievably locked on contract

Tools Used

  • Foundry

  • Manual Review

Recommendations

Don't reference the contract's balance in internal accounting. Replace with a variable that separately tracks contest funds.

...
uint256 private contestFunds = 0;
...
function selectWinner(){
...
contestFunds += totalFees;
...
}
...
function withdrawFees() external {
require(contestFunds == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
contestFunds = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
Updates

Lead Judging Commences

Hamiltonite Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

greifers-send-money-to-contract-to-block-withdrawfees

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.