Beatland Festival
First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

ETH transfer via `.transfer()` may revert for contracts needing more than 2300 gas

fredo182

Root + Impact

ETH transfer via .transfer() may revert for contracts needing more than 2300 gas

Description

The FestivalPass::withdraw function sends the contarct's entire ETH balance to a specified adderss using Solidity's transfer method. transfer only forwads 2300 gas to the recipient, which is insufficient for most smat contracts to receive funds unless they're extremely minimal. In case target is a contract that requires more than 2300 gas in its fallback or receive function the call will revert, and ETH will be stuck in the FestivalPass contract.

function withdraw(address target) external onlyOwner {
@> payable(target).transfer(address(this).balance); // unsafe fixed-gas transfer
}

Risk

Likelihood: Low
Occurs when target is a contract with a non-trivial receive() or fallback() function.

Impact: Low
Not able to withdraw funds to a contract address.

Proof of Concept

function test_revertOnWithdraw() public {
vm.deal(address(festivalPass), 1 ether);
assertEq(address(festivalPass).balance, 1 ether);
​
GasHog gasHog = new GasHog();
​
vm.prank(owner);
vm.expectRevert();
festivalPass.withdraw(address(gasHog));
​
assertEq(address(festivalPass).balance, 1 ether);
}

Recommended Mitigation

Use call{value: ...}("") instead of transfer() to allow forwarding all remaining gas and correctly andle contract recipients.

- payable(target).transfer(address(this).balance);
+ (bool success, ) = payable(target).call{value: address(this).balance}("");
+ require(success, "ETH withdrawal failed");
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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