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

Incorect Access Control

sageceo

Root + Impact

Description

  • The Withdraw function suppose to have only organizer Access control as stated in the comment above.

  • The withdraw function has "onlyOwner" modifier which is not the right acces control of the function. The correct modifier is "onlyOrganizer".

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

Risk

Likelihood:

  • Reason 1 // Likely to happen in contracts where multiple roles (e.g., owner, admin, organizer) are used.

  • Reason 2 // Can go unnoticed unless unit tests or access control verification is thorough.

Impact:

  • Impact 1 // If the organizer is not the contract owner, they will be unable to perform necessary withdrawals, resulting in loss of availability.

  • Impact 2 // Misalignment between comments and access logic can confuse auditors and developers, increasing the risk of future vulnerabilities.

  • Impact 3 // There is no onlyOwner modifier in the contract. this may lead to DOC or Any one can call the withdraw function

Proof of Concept

Verifies that the organizer is not the owner.

Simulates the organizer trying to call withdraw, expecting it to fail due to incorrect modifier (onlyOwner).

Confirms that access control is misaligned with business logic.

function testOnlyOwnerCanWithdrawButOrganizerCannot() public {
address owner = contractInstance.owner();
address organizer = contractInstance.organizer();
​
// Ensure owner is not organizer
assert(owner != organizer);
​
// Simulate organizer trying to withdraw
vm.prank(organizer);
vm.expectRevert("Ownable: caller is not the owner");
contractInstance.withdraw(organizer);
}
​

Recommended Mitigation

Replace onlyOwner with onlyOrganizer to align with function purpose and comment.
Ensure onlyOrganizer is properly implemented:

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

Lead Judging Commences

inallhonesty Lead Judge 28 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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