Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Loss of Funds Due to Invalid Balance Check in withdrawFees Function

Summary

The withdrawFees function in the provided code is vulnerable to a loss of all fees due to a condition check on the contract's balance.

Vulnerability Details

The vulnerability arises from the condition check in the withdrawFees function:

require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

If an attacker performs a self-destruct operation on a self-made contract with some ether in it, it will change the address(this).balance value (because contract won’t be able to refuse money), making the equality check never true. As a result, the fees collected in the contract will be locked forever.

Impact

The impact of this vulnerability is the loss of all fees collected in the contract. Since the distribution of the prize is calculated based solely on the product of the number of players and the entrance fee, 20% of any additional fees sent to the contract will remain locked, as the condition check in the withdrawFees function will never pass.

It does not impact the selectWinner() function, so 80% of funds will always be distributed to each winner, same for the NFT.

Tools Used

Manual review.

Recommendations

To mitigate this vulnerability, consider implementing the following measures:

  1. Purpose of this check is to know if there are currently player active. So check players.length == 0 instead of using address(this).balance.

  2. Alternatively, implement a lock to know when the raffle is running or not.

Updates

Lead Judging Commences

Hamiltonite Lead Judge over 1 year 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.