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

Bad design when withdrawing ETH could create delays due neccessary communication between organizer & owner plus a potential risk of losing funds

ariiellus

Root + Impact

Description

  • The withdraw() function comment declares "Organizer withdraws ETH" but the function uses the onlyOwner modifier, preventing the organizer from accessing the funds despite being responsible to manage the event. which represents either a bad design or a small error when writing the contract

  • In addition, the target parameter allows funds to be sent to any address without validation, creating a potential scenario of accidental fund loss due to incorrect address input.

https://github.com/CodeHawks-Contests/2025-07-beatland-festival/blob/5034ccf16e4c0be96de2b91d19c69963ec7e3ee3/src/FestivalPass.sol#L147

Risk

Likelihood:

  • Reason 1 // When the organizer attempts to withdraw ETH he will not be able to do so and will need to coordinate with the contract owner to withdraw the funds.

  • Reason 2 // Whoever is responsible to call the function can set incorrectly the destination address when calling the function.

Impact:

  • Impact 1: Due both organizer / owner are trusted, this design creates delays when the organizer needs the funds

  • Impact 2: An incorrect destination address may result in loss of funds.

Proof of Concept

I'm adding two modifications of a current test in the test suite. One for a case when the Organizer tries to withdraw the ETH and another if an incorrect address is input.

// Case A, Organizer tries to withdraw ETH
function test_Withdraw_Success() public {
// Users buy passes
vm.prank(user1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
vm.prank(user2);
festivalPass.buyPass{value: VIP_PRICE}(2);
uint256 expectedBalance = GENERAL_PRICE + VIP_PRICE;
assertEq(address(festivalPass).balance, expectedBalance);
// Organizer withdraws
uint256 organizerBalanceBefore = organizer.balance;
vm.prank(organizer); // modified to be the organizer who calls the function. It will fail.
festivalPass.withdraw(organizer);
assertEq(organizer.balance, organizerBalanceBefore + expectedBalance);
assertEq(address(festivalPass).balance, 0);
}
​
// Case B, Organizer inputs a wrong direction
function test_Withdraw_Success() public {
// Users buy passes
vm.prank(user1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
vm.prank(user2);
festivalPass.buyPass{value: VIP_PRICE}(2);
uint256 expectedBalance = GENERAL_PRICE + VIP_PRICE;
assertEq(address(festivalPass).balance, expectedBalance);
// Organizer withdraws
uint256 organizerBalanceBefore = organizer.balance;
vm.prank(organizer); // assuming organizer is who is able to call the function
festivalPass.withdraw(0xdead); // funds are lost forever
assertEq(organizer.balance, organizerBalanceBefore + expectedBalance);
assertEq(address(festivalPass).balance, 0);
}

Recommended Mitigation

Set the onlyOrganizer modifier instead onlyOwner

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

Lead Judging Commences

inallhonesty Lead Judge 27 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.