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

Stuck fees due to ETH accounting issues DOS withdraw

aballok

Summary

Strict equality check for fees can lead to inability

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

withdrawal of fees require strict equality of totalFees accounting and actual balance

This can easily be problematic if due to the following

  1. Present ETH before account was deployed

  2. ETH sent by miners/validators as coinbase transaction

  3. ETH sent via selfdestruct

This implies address(this).balance >= uint256(totalFees)

Impact

Function many neve be able to be called if extra ETH is sent into the account, resulting in the fees being locked in the contracts

Tools Used

Manual Analysis

Recommendations

Recommended to not use strict equality and just require >= and or to not rely on address(this).balance or any other suitable logical solution to prevent this vulnerability

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 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.

Give us feedback!