Incorrect FestivalPass.passSupply accounting in FestivalPass.configurePass() leads to an oversupply of passes
Description
The FestivalPass.configurePass() function is used to configure the price and max supply of a pass, allowing the organizer to reduce or increase the price/max supply of a pass.
However the FestivalPasson.configurePass() function incorrectly resets the supply counter (passSupply) for a given pass type to zero.
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;
}
As a result, the number of new passes that can be minted is equal to the new maxSupply, in addition to those already in circulation, breaking one of the contract's core invariants.
Risk
Likelihood:
This issue occurs when configuring a pass using FestivalPass.configurePass() when the circulating supply of that pass is > 0
Impact:
The number of mints of a pass will exceed the max supply of that pass, causing oversupply.
Proof of Concept
Append the following poc to FestivalPass.t.sol and run it using forge test --mt test_ConfigurePass_AllowsMoreMintsThanMaxSupply
function test_ConfigurePass_AllowsMoreMintsThanMaxSupply() public {
uint256 generalPassesMinted = 0;
for(uint256 i = 0; i < GENERAL_MAX_SUPPLY; i++) {
address randomAddr = makeAddr(string(abi.encodePacked(generalPassesMinted)));
vm.deal(randomAddr, GENERAL_PRICE);
vm.prank(randomAddr);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
generalPassesMinted++;
vm.stopPrank();
}
assertEq(festivalPass.passSupply(1), GENERAL_MAX_SUPPLY);
vm.startPrank(organizer);
uint256 newMaxSupply = GENERAL_MAX_SUPPLY + 5;
festivalPass.configurePass(1, GENERAL_PRICE, newMaxSupply);
vm.stopPrank();
for(uint256 i = 0; i < newMaxSupply; i++) {
address randomAddr = makeAddr(string(abi.encodePacked(generalPassesMinted)));
vm.deal(randomAddr, GENERAL_PRICE);
vm.prank(randomAddr);
festivalPass.buyPass{value: GENERAL_PRICE}(1);
generalPassesMinted++;
vm.stopPrank();
}
assertEq(generalPassesMinted, GENERAL_MAX_SUPPLY + newMaxSupply);
}
Recommended Mitigation
Consider removing the line that resets the passSupply. Additionally, add a check to ensure that the new maxSupply is more than the number of passes already minted.
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(maxSupply > passSupply[passId], "Max supply must be greater than the current pass supply");
passPrice[passId] = price;
passMaxSupply[passId] = maxSupply;
- passSupply[passId] = 0; // Reset current supply
}