Beatland Festival

First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

[H-1] Unsecured Direct Transfer in `FestivalPass:withdraw` function.


Description

The `withdraw` function in the `FestivalPass` contract allows the contract owner to transfer all ETH
collected to a specified address. However, the implementation uses direct `transfer()`,
which is not gas-safe and can fail unexpectedly due to the fixed 2300 gas stipend limit imposed by transfer().
```javascript
function withdraw(address target) external onlyOwner {
payable(target).transfer(address(this).balance);
}
```
Direct ETH transfers using `.transfer()` are considered unsafe since EIP-1884,
which increased the gas cost of certain opcodes and may cause transfer() to fail on some smart contract wallets like multisigs.

Risk

Impact:

Risk: DoS (Denial of Service to withdraw funds), lost funds access
1. Contract owner could lose access to funds if transfer() fails due to recipient gas usage.
2. Funds could be locked forever if recipient is a contract not compatible with transfer().

Proof of Concept

Deploy `FestivalPass` and call `withdraw()` using a recipient address that has a fallback function consuming more than 2300 gas:
Add follwing in `FestivalPass.t.sol`
```javascript
contract FallbackRevert {
fallback() external payable {
require(gasleft() > 10000, "Consumes too much gas");
}
}
```
and test following test, it will be reverted due to more gas error.
```javascript
function testWithdrawRevertsOnGasHeavyFallback() public {
// Deploy fallback contract
FallbackRevert br = new FallbackRevert();
// Fund festival contract by buying passes
vm.prank(user1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
// Attempt withdraw
vm.prank(owner);
vm.expectRevert(); // .transfer should revert
festivalPass.withdraw(address(br));
}
```
- Call withdraw(address(receiver)) from owner.
- Transaction will revert due to .transfer() failing to deliver ETH.

Recommended Mitigation

Use a low-level call with manual success check, which avoids the 2300 gas limit problem:
```diff
function withdraw(address target) external onlyOwner {
+ require(target != address(0), "Invalid address");
- payable(target).transfer(address(this).balance);
+ uint256 balance = address(this).balance;
+ (bool success, ) = payable(target).call{value: balance}("");
+ require(success, "Withdraw failed");
}
```
Why this is better:
~ call forwards all available gas (unless specified otherwise).
~ More compatible with smart contract wallets.
~ Can safely handle larger logic in the recipient contract.
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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