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

[H-2] Owner renounce of FestivalPass NFT can set owner contract to address(0)

emerjux

Root + Impact

Description

Because FestivalPass NFT is also Ownable2Step and Ownable, any owner can call the public function renounceOwnership() and set the contract owner's address to address(0).

The function renounceOwnership() is inherited via OpenZeppelin::Ownable, it is true that it can only be executed by the owner at the moment, but if someone executes it, then it will render the BeatToken unavailable.

function renounceOwnership() public virtual onlyOwner {
_transferOwnership(address(0));
}

Risk

Likelihood:

  • Owner of the NFT decides to renounce of ownership due to disgrunted situation

  • Owner's private key is compromised and the attacker issues the renounce ownership public function

Impact:

  • The whole NFT festival becomes innoperable for all admin actions.

Proof of Concept

Just set at the beginning of setUp() add the line festivalPass.renounceOwnership(); just after the creation of the new FestivalPass contract

function setUp() public {
owner = address(this);
organizer = makeAddr("organizer");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
// Deploy contracts
beatToken = new BeatToken();
festivalPass = new FestivalPass(address(beatToken), organizer);
festivalPass.renounceOwnership(); // owner renounces of ownership
// Set festival contract in BeatToken
beatToken.setFestivalContract(address(festivalPass));
// Configure passes as organizer
vm.startPrank(organizer);
festivalPass.configurePass(1, GENERAL_PRICE, GENERAL_MAX_SUPPLY);
festivalPass.configurePass(2, VIP_PRICE, VIP_MAX_SUPPLY);
festivalPass.configurePass(3, BACKSTAGE_PRICE, BACKSTAGE_MAX_SUPPLY);
vm.stopPrank();
// Fund test users
vm.deal(user1, 10 ether);
vm.deal(user2, 10 ether);
}

Recommended Mitigation

Override the function FestivalPass::renounceOwnership() so it will always fall back to the current owner.

+ function renounceOwnership() public virtual override onlyOwner {
+ _transferOwnership(owner());
+ }
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.