Beatland Festival

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

[ M-1 ] - Off-by-One Error in Memorabilia Supply Check

Off-by-One Error in Memorabilia Supply Check + Fewer NFTs Than Intended

Description

  • Normally, the redeemMemorabilia function should allow users to mint up to the full maxSupply of memorabilia items in a collection.

  • In this implementation, the function checks require(collection.currentItemId < collection.maxSupply, "Collection sold out"). Since currentItemId starts at 1 and is incremented after minting, this check prevents minting the final item, so only maxSupply - 1 items can ever be minted.

function redeemMemorabilia(uint256 collectionId) external {
MemorabiliaCollection storage collection = collections[collectionId];
require(collection.priceInBeat > 0, "Collection does not exist");
require(collection.isActive, "Collection not active");
@> require(collection.currentItemId < collection.maxSupply, "Collection sold out");
// ... mint logic ...
}

Risk

Likelihood:

  • This will occur every time a memorabilia collection is created and users attempt to mint up to the maximum supply.

  • Any collection with a nonzero max supply will be affected, so the issue is present in all normal usage.

Impact:

  • Users will be unable to mint the final item in each collection, resulting in fewer NFTs than intended.

  • This can lead to user confusion, failed transactions, and failure to meet advertised collection goals.

Proof of Concept

To reproduce this issue, copy and paste the following test code into your test file (e.g., test/contract.t.sol). This test creates a collection with a max supply of 3, but only 2 items can be minted before the function reverts:

function test_OffByOneErrorInMemorabiliaSupply() public {
// Create a collection with max supply of 3
vm.prank(organizer);
uint256 collectionId = festivalPass.createMemorabiliaCollection(
"Test Collection",
"ipfs://QmTest",
1e18,
3, // maxSupply = 3
true
);
vm.prank(address(festivalPass));
beatToken.mint(user1, 10e18);
vm.startPrank(user1);
festivalPass.redeemMemorabilia(collectionId);
assertEq(festivalPass.balanceOf(user1, festivalPass.encodeTokenId(collectionId, 1)), 1);
festivalPass.redeemMemorabilia(collectionId);
assertEq(festivalPass.balanceOf(user1, festivalPass.encodeTokenId(collectionId, 2)), 1);
vm.expectRevert("Collection sold out");
festivalPass.redeemMemorabilia(collectionId);
vm.stopPrank();
assertEq(festivalPass.balanceOf(user1, festivalPass.encodeTokenId(collectionId, 1)), 1);
assertEq(festivalPass.balanceOf(user1, festivalPass.encodeTokenId(collectionId, 2)), 1);
}

Explanation:
This test shows that, despite a max supply of 3, only 2 items can be minted. The third attempt reverts with "Collection sold out", proving the off-by-one error.

Recommended Mitigation

Change the strict inequality (<) to a non-strict inequality (<=) so the final item can be minted.

- require(collection.currentItemId < collection.maxSupply, "Collection sold out");
+ require(collection.currentItemId <= collection.maxSupply, "Collection sold out");
Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Off by one error in redeemMemorabilia

Support

FAQs

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