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

Possibility of DOS in withdrawFees()

Summary

withdrawFees() is used to transfer the accumulated fees to the feeAddress. However, a notable issue is that the function always reverts if anyone sends ether directly to the contract

Vulnerability Details

withdrawFees() has a require(address(this).balance == uint256(totalFees) to verify the presence of active players. The issue arises when someone sends only 1 wei to the contract, preventing the require condition from being met. This situation can lead to DOS attack, effectively blocking the funds within the contract. The vulnerability arises from the use of address(this).balance in the comparison, and an attacker can exploit this by creating a malicious contract with a selfdestruct() pointing to the contract, ensuring that the require condition is never satisfied.

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

Impact

DOS provocating a lock of funds.

Tools Used

Manual review

Recommendations

Instead of using address(this).balance use a virtual balance to store the balance of the contract.

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.