Beatland Festival

First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

`configurePass` when `buyPass` is on-going will cause current supply to return expected results

configurePass when buyPass is on-going will cause current supply to return expected results

Description

  • It seems there exists an assumption that buyPass should be called only after configurePass is called, so the configs of the festival pass like price and max supply are set before the sale starts

  • There is no hard restriction in configurePass to limit organizer from setting new configs WHILE users are still buying the pass, so the current supply can be reset to 0 in the middle of other users buying the festival pass, which can result in different users buying in different price or max supply, and current supply can be reset even after the same festival pass is already selling to users

function configurePass(
uint256 passId,
uint256 price,
uint256 maxSupply
) external onlyOrganizer {
require(passId == GENERAL_PASS || passId == VIP_PASS || passId == BACKSTAGE_PASS, "Invalid pass ID");
require(price > 0, "Price must be greater than 0");
require(maxSupply > 0, "Max supply must be greater than 0");
passPrice[passId] = price;
passMaxSupply[passId] = maxSupply;
@> passSupply[passId] = 0; // Reset current supply //@audit: what if you call configurePass when a pass is already sold out i.e. maxSupply is already reached
}

Risk

Likelihood:

  • The issue will arise whenever organizer calls configurePass while any users are still buying the festival pass

Impact:

  • The max supply and price of the festival pass can be updated any time even other users are still buying it, the current supply will also get reset, so when organizer can configurePass without hard restirction from the contract will cause the current supply storage for the same passId won't add up and would hugely affect the accouting of the pass and affects other business logic given the initial assumption of correct price, max supply and current supply for the respective festival pass

Proof of Concept

The current supply will be reset to 0 if organizer configurePass after a user buys a pass, but in fact it should be 2 and this could cause a lot of confusions.

uint passId = 1;
uint price = 1 ether;
uint maxSupply = 1000;
vm.prank(organizer);
festivalPass.configurePass(passId, price, maxSupply); // called by organizer
vm.prank(user1);
festivalPass.buyPass{value: price}(passId); // Alice buys a pass
assertEq(festivalPass.passSupply(passId), 1); // current supply being 1
vm.prank(organizer);
festivalPass.configurePass(passId, 2 ether, 1001); // organizer configures the price and max supply again
vm.prank(user2);
festivalPass.buyPass{value: 2 ether}(passId); // Bob buys a pass
assertEq(festivalPass.passSupply(passId), 1); // current supply got reset and it becomes 1 again

Recommended Mitigation

Either a require checking can be added to configurePass to restrict organizer from being able to configure only when a pass hasn't been bought yet, or even adding a enum to the contract to indicate different stages for the sale of the pass to ensure the flow is consistent and no unexpected configurations can happen during the middle of the sale.

function configurePass(
uint256 passId,
uint256 price,
uint256 maxSupply
) external onlyOrganizer {
require(passId == GENERAL_PASS || passId == VIP_PASS || passId == BACKSTAGE_PASS, "Invalid pass ID");
require(price > 0, "Price must be greater than 0");
require(maxSupply > 0, "Max supply must be greater than 0");
+ require(passSupply[passId] == 0, "Can configure only if a pass hasn't been bought")
passPrice[passId] = price;
passMaxSupply[passId] = maxSupply;
passSupply[passId] = 0; // Reset current supply //@audit: what if you call configurePass when a pass is already sold out i.e. maxSupply is already reached
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 26 days ago
Submission Judgement Published
Validated
Assigned finding tags:

configurePass resets the current pass supply circumventing the max supply check

This is not acceptable as high because any attack vectors related to organizer trying to milk ETH from participants is voided by the fact that the organizer is trusted.

Support

FAQs

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