Off-by-One in redeemMemorabilia Prevents Last NFT From Being Redeemed
Description
-
The createMemorabiliaCollection function allows an organizer to create an NFT collection that can be exchanged for the BEAT token via the redeemMemorabilia function by users.
-
The redeemMemorabilia function checks if collection.currentItemId is less than collection.maxSupply. However, the currentItemId starts with 1 in the createMemorabiliaCollection function. This prevents the final item (where currentItemId equals maxSupply) from being redeemed.
function createMemorabiliaCollection(
string memory name,
string memory baseUri,
uint256 priceInBeat,
uint256 maxSupply,
bool activateNow
) external onlyOrganizer returns (uint256) {
require(priceInBeat > 0, "Price must be greater than 0");
require(maxSupply > 0, "Supply must be at least 1");
require(bytes(name).length > 0, "Name required");
require(bytes(baseUri).length > 0, "URI required");
uint256 collectionId = nextCollectionId++;
collections[collectionId] = MemorabiliaCollection({
name: name,
baseUri: baseUri,
priceInBeat: priceInBeat,
maxSupply: maxSupply,
@> currentItemId: 1,
isActive: activateNow
});
emit CollectionCreated(collectionId, name, maxSupply);
return collectionId;
}
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");
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
tokenIdToEdition[tokenId] = itemId;
_mint(msg.sender, tokenId, 1, "");
emit MemorabiliaRedeemed(msg.sender, tokenId, collectionId, itemId);
}
Risk
Likelihood:
Impact:
Proof of Concept
The following test shows a user trying to redeem the 10th NFT in one collection. Running forge test --mt test_Audit_RedeemMaxSupply -vv shows the output that the 10th redemption is reverted due to the sold out.
function test_Audit_RedeemMaxSupply() public {
vm.prank(organizer);
uint256 maxSupply = 10;
uint256 collectionId = festivalPass.createMemorabiliaCollection(
"Future Release",
"ipfs://QmFuture",
10e18,
maxSupply,
true
);
vm.startPrank(address(festivalPass));
beatToken.mint(user1, 10000e18);
vm.stopPrank();
vm.startPrank(user1);
for (uint256 i = 0; i < maxSupply - 1; i++) {
festivalPass.redeemMemorabilia(collectionId);
console.log("Redeem sucess:", i + 1);
}
vm.expectRevert("Collection sold out");
festivalPass.redeemMemorabilia(collectionId);
console.log("Redeem reverted:", maxSupply);
vm.stopPrank();
}
Recommended Mitigation
Modify the supply check in redeemMemorabilia to use <= (less than or equal to) instead of <, ensuring that the final item can be redeemed. This approach is preferable to modifying the createMemorabiliaCollection function (which is clearly documented to start currentItemId at 1).
// Redeem a memorabilia NFT from a collection
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");
+ require(collection.currentItemId <= collection.maxSupply, "Collection sold out"); // allow equals
// Burn BEAT tokens
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
// Generate unique token ID
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
// Store edition number
tokenIdToEdition[tokenId] = itemId;
// Mint the unique NFT
_mint(msg.sender, tokenId, 1, "");
emit MemorabiliaRedeemed(msg.sender, tokenId, collectionId, itemId);
}
