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

Important checks for zero address are skipped enabling misconfiguration and unexpected failures.

fredo182

Root + Impact

Important checks for zero address are skipped enabling misconfiguration and unexpected failures.

Description

Smart contracts should validate that critical address inputs are not the zero address. These include roles with permissions and dependencies.

In the FestivalPass and BeatToken contracts, multiple assignments to critical roles and dependecies lack zero address checks. This creates opportunities of misconfiguration and unexpected failures.

// src/FestivalPass.sol
@> beatToken = _beatToken;
​
// src/FestivalPass.sol
@> organizer = _organizer;
​
// src/BeatToken.sol
@> festivalContract = _festival;

Additional, there is a transfer of ETH that lacks zero address validation and can result in loss of funds.

// src/FestivalPass.sol
@> payable(target).transfer(address(this).balance);

Risk

Likelihood: Medium

The problem may occur during deployment, when owner calls set functions or while withdrawing the funds of festival proceeds.

Impact: High

The impact varies per case:

Setting the beat token during deployment requires new deployment and additional costs.
Withdrawing ETH to zero address results in irreversible loss of funds.
Call setters can be done again with the correct address and small disruption of the protocol.

Proof of Concept

function test_withdrawToZeroAddress() public {
vm.prank(user1);
vm.expectEmit(true, true, false, true);
emit PassPurchased(user1, 1);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
​
assertEq(address(festivalPass).balance, GENERAL_PRICE);
assertEq(address(0).balance, 0);
​
vm.prank(owner);
festivalPass.withdraw(address(0));
​
assertEq(address(festivalPass).balance, 0);
assertEq(address(0).balance, GENERAL_PRICE);
}

Recommended Mitigation

Given that it is known that some checks where left out for gas efficiency, the recommended mitigations below capture only the irreversible

- beatToken = _beatToken;
+ require(_beatToken != address(0), "Invalid token address");
+ beatToken = _beatToken;
​
- payable(to).transfer(address(this).balance);
+ require(to != address(0), "Cannot withdraw to zero address");
+ payable(to).transfer(address(this).balance);
Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Zero address check

Owner/admin is trusted / Zero address check - Informational

Support

FAQs

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