Beatland Festival
AI First Flight #4
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: medium
Invalid

Unchecked Transfer Success and Incorrect Access Control in `withdraw()` Lead to Permanent Fund Locking

mafa

Unchecked Transfer Success and Incorrect Access Control in withdraw() Lead to Permanent Fund Locking

Description

The withdraw() function contains multiple critical design issues.First, the use of the onlyOwner modifier instead of onlyOrganizer prevents the legitimate event operator from accessing ticket revenue and introduces an unnecessary centralization risk.Additionally, the function does not validate that the withdrawal target is not the zero address, which may result in the irreversible loss of funds.

Most critically, the function uses transfer() to send ETH. Since transfer() enforces a strict 2,300 gas stipend, any withdrawal to a smart contract wallet (e.g., a multi-signature wallet) whose receive() or fallback() function requires more gas will consistently revert.

// Organizer withdraws ETH
@> function withdraw(address target) external onlyOwner {
@> payable(target).transfer(address(this).balance);
​
}

Risk

Likelihood: Medium to High. Since many professional organizers and institutional users employ multi-signature or smart contract wallets, the 2300 gas limit of transfer() will frequently trigger reverts, and the role confusion between Owner and Organizer is a certain operational bottleneck.

Impact: High. This flaw leads to a total loss of access for the Organizer and the potential for funds to be permanently locked in the contract or accidentally sent to a zero address, resulting in a complete loss of protocol revenue.

Proof of Concept

pragma solidity 0.8.25;
​
import {Test, console} from "forge-std/Test.sol";
import {FestivalPass} from "../src/FestivalPass.sol";
import {BeatToken} from "../src/BeatToken.sol";
import {FestivalPassTest} from "./FestivalPass.t.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
​
contract AuditTest is FestivalPassTest {
MultiSigWallet multiSigWallet;
​
function setUp() public override {
super.setUp();
multiSigWallet = new MultiSigWallet();
}
​
function testOrganizerCannotWithdraw() public{
vm.expectRevert(abi.encodeWithSelector(
Ownable.OwnableUnauthorizedAccount.selector,
organizer
));
vm.prank(organizer);
festivalPass.withdraw(address(multiSigWallet));
}
​
function testWithdrawRevert() public {
​
vm.expectRevert();
vm.prank(owner);
festivalPass.withdraw(address(multiSigWallet));
}
​
}
​
contract MultiSigWallet {
mapping(address => uint256) public totalReceivedPerSender;
uint256 public transactionCount;
​
event Deposit(address indexed sender, uint256 amount, uint256 indexed txId);
receive() external payable {
transactionCount++;
totalReceivedPerSender[msg.sender] += msg.value;
emit Deposit(msg.sender, msg.value, transactionCount);
}
}
​

Execution Trace for testWithdrawRevert():

├─ [11983] FestivalPass::withdraw(MultiSigWallet: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
│ ├─ [2300] MultiSigWallet::receive{value: 10000000000000000000}()
│ │ └─ ← [OutOfGas] EvmError: OutOfGas
│ └─ ← [Revert] EvmError: Revert
└─ ← [Revert] EvmError: Revert

Recommended Mitigation

Change the access modifier from onlyOwner to onlyOrganizer to correct the permission logic. Add a require statement to prevent sending funds to a zero address. Finally, replace transfer() with a low-level call() and check its return value to ensure compatibility with smart contract wallets and to handle transfer failures correctly.

- function withdraw(address target) external onlyOwner {
+ function withdraw(address target) external onlyOrganizer {
+ require(target != address(0), "Withdraw: target is zero address");
- payable(target).transfer(address(this).balance);
+ (bool success, ) = payable(target).call{value: amount}("");
+ if (!success) {
+ revert Withdrawal__TransferFailed();
+ }
​
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!