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

feeAddress will be Unable to receive any fees if ether is sent directly to the contract.

sm4rty

Summary

An attacker can prevent the withdrawal of fees, potentially disrupting the fee distribution mechanism.

Vulnerability Details

The withdrawFees() function is defined as follows:

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

The contract assumes that the total fees are equal to the contract's balance to allow the withdrawal of fees to the feeAddress. However, this assumption is not guaranteed, as there are scenarios where the contract's balance might not match the total fees.

One such scenario is if anyone sends ether directly to the contract through a self-destruct function or other means. When ether is sent directly to the contract without interacting with the enterRaffle or selectWinner functions, the balance of the contract increases, but the total fees (totalFees) remain unaffected. As a result, the check in the withdrawFees function will fail, and the call to transfer fees to the feeAddress will be reverted.

This situation can leave the feeAddress unable to receive any fees, creating a loss of funds for the contract owner.

Impact

The vulnerability could result in the feeAddress being unable to receive any fees, leading to a disruption in the contract's intended fee distribution.

Tools Used

Manual Analysis

Recommendations

To mitigate this vulnerability, the contract should change the check from equality (==) to greater than or equal to (>=).

From:

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

To:

require(address(this).balance >= uint256(totalFees), "PuppyRaffle: There are currently players active!");
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.