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

FestivalPass.sol - Withdraw Access Control Contradicts Documentation

0xrektified

Description

The withdraw function's access control implementation contradicts multiple documentation sources that clearly state the organizer should withdraw funds, but the implementation uses onlyOwner instead of onlyOrganizer, creating confusion about roles and responsibilities.

Root Cause

Multiple documentation sources specify that the organizer withdraws funds, but the implementation uses wrong access control:

Interface Documentation (line 74-78):

/**
* @notice Emitted when the organizer withdraws collected funds
* @param organizer Address of the organizer
* @param amount Amount of ETH withdrawn
*/
event FundsWithdrawn(address indexed organizer, uint256 amount);

Code Comments (line 156-157):

// Organizer withdraws ETH
function withdraw(address target) external onlyOwner {

README.md (line 17):

Owner: The owner and deployer of contracts, sets the Organizer address, collects the festival proceeds.

Risk

Likelihood: High - Every withdrawal operation will use the wrong access control pattern.

Impact: Low - Functional confusion and role misalignment, but no direct security risk since owner is trusted.

Impact

  • Documentation inconsistency creates confusion about system roles

  • Interface specifications contradict implementation behavior

  • Could lead to integration errors where external systems expect organizer initiated withdrawals

Proof of Concept

This test demonstrates the access control contradiction where documentation specifies organizer withdrawals but implementation requires owner privileges.

function test_WithdrawAccessControlMismatch() public {
// Fund the contract with pass sales
vm.prank(user1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
// Documentation states organizer should withdraw, but this fails
vm.prank(organizer);
vm.expectRevert(); // Fails because function requires onlyOwner
festivalPass.withdraw(organizer);
// Only owner can withdraw, contradicting documentation
vm.prank(owner);
festivalPass.withdraw(organizer); // This works
// Event would incorrectly show organizer as withdrawer if it were emitted
// But organizer didn't actually call the function
}

Recommended Mitigation

Align the implementation with documentation by changing the access control:

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

Alternative: Update all documentation to reflect that the owner handles withdrawals:

/**
- * @notice Emitted when the organizer withdraws collected funds
- * @param organizer Address of the organizer
+ * @notice Emitted when the owner withdraws collected funds
+ * @param target Address of the owner send the withdraw to
* @param amount Amount of ETH withdrawn
*/
- event FundsWithdrawn(address indexed organizer, uint256 amount);
+ event FundsWithdrawn(address indexed target, uint256 amount);

Consistency between documentation and implementation is crucial for proper integration and understanding of system roles.

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.