Beatland Festival

AI First Flight #4
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

withdraw() function is restricted to contract owner instead of organizer, contradicting code comment and intended design

Description

The withdraw() function allows withdrawal of all ETH collected from pass sales, but it is protected by onlyOwner instead of onlyOrganizer:

solidity

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

The inline comment explicitly states "Organizer withdraws ETH", indicating the intended design is that the festival organizer — not the contract deployer/owner — should control fund withdrawal.

However, due to the use of onlyOwner (inherited from OpenZeppelin's Ownable2Step), only the contract owner (set at deployment and transferable via ownership functions) can call this function.

This creates a critical mismatch between documented intent and actual access control.

Impact

  • Medium severity: No immediate loss of funds, but significant centralization and operational risk.

  • Misaligned incentives and trust model:

    • The protocol distinguishes between owner (likely team/multisig) and organizer (event-specific entity).

    • Festival organizers expect to withdraw revenue from ticket sales.

    • Currently, they cannot — only the contract owner can.

  • Operational failure risk:

    • If ownership is not transferred to the organizer (or a shared multisig), the organizer has no access to earned funds.

    • If ownership is transferred, the team permanently loses control over emergency recovery.

  • Trust assumption violation: Users buying passes assume revenue goes to the event organizer, but funds are locked under a different role.

  • Documentation vs code discrepancy: Clear comment-code mismatch reduces auditability and increases likelihood of misuse.

While not a direct exploit, this is a serious design flaw in access control that can lead to locked funds or unintended centralization.

Proof of Concept

Simple scenario demonstrating the mismatch:

solidity

function test_WithdrawAccessibleOnlyToOwnerNotOrganizer() public {
// Deploy: msg.sender (this test) becomes owner
// organizer is set to different address in constructor
assertEq(festival.owner(), address(this));
assertNeq(festival.organizer(), address(this));
// Some passes are sold → contract has ETH balance
vm.deal(address(0xAAAA), 1 ether);
vm.prank(address(0xAAAA));
festival.buyPass{value: 1 ether}(GENERAL_PASS);
assertGt(address(festival).balance, 0);
// Organizer tries to withdraw → REVERTS
vm.prank(festival.organizer());
vm.expectRevert("Ownable: caller is not the owner");
festival.withdraw(festival.organizer());
// Owner (not organizer) can withdraw → succeeds
festival.withdraw(festival.organizer());
assertEq(address(festival).balance, 0);
}

This test passes and shows the organizer cannot access funds despite the comment claiming they can.

Recommended Mitigation

Align the access control with the documented intent. Two clean options:

Option 1: Change modifier to onlyOrganizer (Recommended)

solidity

// Organizer withdraws ETH
function withdraw(address target) external onlyOrganizer {
payable(target).transfer(address(this).balance);
}

This matches the comment and design intent: the festival organizer controls revenue.

Option 2: Keep onlyOwner but update comment and docs

If the team intentionally wants the owner (not organizer) to control funds, then:

  • Remove or correct the misleading comment

  • Update all documentation to reflect that the contract owner withdraws funds

Recommendation: Go with Option 1 — it aligns with the role separation already present in the contract (setOrganizer, onlyOrganizer functions).

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 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!