Trick or Treat

First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: low
Valid

Insecure Use of transfer in withdrawFees Function

Summary

The withdrawFees function utilizes the transfer method to send ETH to the contract owner. The contract uses built-in transfer() function for transferring tokens.

The transfer() function was commonly used in earlier versions of Solidity for its simplicity and automatic reentrancy protection. However, it was identified as potentially problematic due to its fixed gas limit of 2300.

Vulnerability Details

The withdrawFees function is designed to enable the contract owner to withdraw all accumulated ETH from the contract. Here's a closer examination of how it operates and the inherent vulnerabilities associated with its current implementation:

payable(owner()).transfer(balance);

The transfer method sends a fixed amount of 2300 gas to the recipient, which is typically sufficient only for simple ETH transfers. If the recipient is a contract that requires more gas to process the incoming ETH (e.g., due to fallback functions or complex logic), the transfer will fail, reverting the entire transaction.

Impact

If the transfer fails, the entire withdrawal process is reverted, leaving the contract's ETH balance inaccessible to the owner. This scenario effectively locks all accumulated fees within the contract indefinitely. The owner cannot retrieve funds essential for maintaining or upgrading the contract, leading to potential stagnation or inability to address future issues.

Tools Used

Manual review

Recommendations

Utilize the call method to send ETH, allowing for dynamic gas management and better handling of transfer outcomes.

function withdrawFees() public onlyOwner nonReentrant {
uint256 balance = address(this).balance;
(bool success, ) = owner().call{value: balance}("");
require(success, "Transfer failed.");
emit FeeWithdrawn(owner(), balance);
}
Updates

Appeal created

bube Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Use of `transfer` instead of `call`

Support

FAQs

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