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

Withdraw function lacks address validation which can lead to permanent loss of funds

lukamm

Withdraw function lacks address validation which can lead to permanent loss of funds

Description

The FestivalPass::withdraw function currently takes in a target address for funds to be withdrawn to. This address is not validated at all, meaning if the owner makes a mistake when calling the function and passes in the wrong target, all funds will be sent to the wrong address and lost or burnt.

function withdraw(address target) external onlyOwner {
@> payable(target).transfer(address(this).balance);
}

Risk

Likelihood:

This will occur when the owner calls the FestivalPass::withdraw function and passes in the incorrect target address.

Impact:

Funds will be withdrawn to wrong address and will be lost or burnt.

Proof of Concept

  1. Passes are configured by the organizer

  2. Users buy passes, increasing the ETH balance of the FestivalPass contract

  3. Owner calls withdraw and passes in the zero address

  4. Funds are sent to the zero address and burnt

Add the following test to the FestivalPass.t.sol file.

function testWithdrawFundsToZeroAddress() public {
// Deal funds to the contract
vm.deal(address(festivalPass), 10 ether);
uint256 balanceBefore = address(festivalPass).balance;
console.log("balanceBefore", balanceBefore);
​
// Withdraw funds to zero address
vm.prank(owner);
festivalPass.withdraw(address(0));
uint256 balanceAfter = address(festivalPass).balance;
console.log("balanceAfter", balanceAfter);
assertEq(balanceAfter, 0);
}

Recommended Mitigation

The recommended mitigation for this is adding a treasuryAddress variable with a setTreasuryAddress function callable by the owner. This function will contain a zero address check. This allows the owner to set the treasuryAddress prior, and if they make a mistake they can simply update the treasuryAddress.

Note: Make sure to add the new event TreasuryAddressUpdated(address newTreasuryAddress) event to the IFestivalPass.sol interface.

address public beatToken;
address public organizer;
+ address public treasuryAddress;
​
​
+ function setTreasuryAddress(address newTreasuryAddress) external onlyOwner {
+ require(newTreasuryAddress != address(0), "Invalid treasury address");
+ treasuryAddress = newTreasuryAddress;
+ emit TreasuryAddressUpdated(newTreasuryAddress);
+ }
​
- function withdraw(address target) external onlyOwner {
- payable(target).transfer(address(this).balance);
- }
+ function withdraw() external onlyOwner {
+ payable(treasuryAddress).transfer(address(this).balance);
+ }
Updates

Lead Judging Commences

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