Beatland Festival

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

[M-01] Access Control Mismatch in FestivalPass::withdraw locks out Organizer

Access Control Mismatch in FestivalPass::withdraw locks out Organizer

Description

  • The code documentation explicitly states that the "Organizer withdraws ETH". However, the withdraw function uses the onlyOwner modifier instead of the onlyOrganizer modifier used elsewhere in the contract (e.g., configurePass).

  • This creates a contradiction between the intended logic (Organizer manages funds) and the actual implementation (only the Contract Owner can move funds).

// Organizer withdraws ETH
// @> Root Cause: Modifier mismatch. Comment says Organizer, code enforces Owner.
function withdraw(address target) external onlyOwner {
payable(target).transfer(address(this).balance);
}

Risk

Likelihood:

  • Certain. The code strictly enforces onlyOwner. The Organizer will fail every time they attempt to call this function (unless organizer == owner, in which case the role distinction is redundant).

Impact:

  • Denial of Service (DoS): The Organizer is unable to access the revenue generated from their ticket sales.

  • Centralization Risk: The Contract Owner holds custody of all funds, requiring the Organizer to trust the Owner to withdraw and forward the money manually.

Proof of Concept

function testOrganizerCannotWithdraw() public {
// 1. Setup: Assume Deployer is Owner, and 'Alice' is Organizer
vm.prank(deployer);
ticketContract.setOrganizer(alice); // Assuming a setter exists
// 2. Fund the contract
vm.deal(address(ticketContract), 10 ether);
// 3. Organizer tries to withdraw per the comments
vm.startPrank(alice);
// 4. Expect Revert because Alice is Organizer, not Owner
vm.expectRevert("Ownable: caller is not the owner");
ticketContract.withdraw(alice);
vm.stopPrank();
}

Explanation

  1. The Setup (setOrganizer)

  • What it does: We make sure we have two distinct users: The Owner (Deployer) and the Organizer (Alice).

  • Why: If the Owner and Organizer are the same person, the bug doesn't exist. We must separate them to prove that the "Organizer" role specifically creates the issue.

  1. Funding (vm.deal)

  • What it does: We give the smart contract 10 ETH.

  • Why: You can't withdraw from an empty contract. We need money in the pot to make the withdrawal attempt realistic.

  1. Impersonation (vm.startPrank(alice))

  • What it does: This is a Foundry cheat code. It tells the test environment: "For all the next lines of code, pretend msg.sender is Alice (the Organizer)."

  • Why: We need to simulate the exact action of the Organizer trying to get their money.

  1. The Assertion (vm.expectRevert)

  • What it does: This is the most critical line. It tells the test runner: "I expect the NEXT line of code to FAIL with this specific error message."

  • Why it proves the bug:

  • If the code was correct (Organizer could withdraw), the transaction would succeed, and this test would fail (because we expected an error but didn't get one).

  • Because the transaction reverts (fails) with "Ownable: caller is not the owner", we have mathematically proven that Alice (the Organizer) is not allowed to withdraw.

Recommended Mitigation

  • Align the code with the documentation. If the Organizer is the intended beneficiary, change the modifier.

- function withdraw(address target) external onlyOwner {
+ function withdraw(address target) external onlyOrganizer {
payable(target).transfer(address(this).balance);
}
Updates

Lead Judging Commences

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