Skip to content

Latest commit

 

History

History
112 lines (79 loc) · 3.65 KB

M-02.md

File metadata and controls

112 lines (79 loc) · 3.65 KB
Raw

Token ID 0 can't be sold using the Sales contracts

In all of the three sales contracts (FixedPrice, LPDA and OpenEdition) a sale can't be configured to be able to include the token id with value 0 by how indices are operated in the buy function.

Using the FixedPrice contract as an example:

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L65-L67

for (uint48 x = sale_.currentId + 1; x <= newId; x++) {
    nft.mint(msg.sender, x);
}

The lowest value sale_.currentId can take during sale creation is 0 (because these are unsigned integers). This means that the lowest minted NFT id is 1 because sale_.currentId + 1 >= 1.

A similar issue happens in the other two contracts LPDA and OpenEdition.

Impact

None of the sales contracts (FixedPrice, LPDA or OpenEdition) can be configured to create a sale that includes the token id 0.

PoC

The following test reproduces the issue using the FixedPrice sale contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;

import "forge-std/Test.sol";
import {FixedPriceFactory} from "src/minters/FixedPriceFactory.sol";
import {FixedPrice} from "src/minters/FixedPrice.sol";
import {OpenEditionFactory} from "src/minters/OpenEditionFactory.sol";
import {OpenEdition} from "src/minters/OpenEdition.sol";
import {LPDAFactory} from "src/minters/LPDAFactory.sol";
import {LPDA} from "src/minters/LPDA.sol";
import {Escher721} from "src/Escher721.sol";

contract AuditTest is Test {
    address deployer;
    address creator;
    address buyer;

    FixedPriceFactory fixedPriceFactory;
    OpenEditionFactory openEditionFactory;
    LPDAFactory lpdaFactory;

    function setUp() public {
        deployer = makeAddr("deployer");
        creator = makeAddr("creator");
        buyer = makeAddr("buyer");

        vm.deal(buyer, 1e18);

        vm.startPrank(deployer);

        fixedPriceFactory = new FixedPriceFactory();
        openEditionFactory = new OpenEditionFactory();
        lpdaFactory = new LPDAFactory();

        vm.stopPrank();
    }
    
    function test_FixedPrice_buy_CantSellTokenId0() public {
        vm.startPrank(creator);

        Escher721 nft = new Escher721();
        nft.initialize(creator, address(0), "Test NFT", "TNFT");

        uint48 startId = 0;
        uint48 amount = 10;
        uint96 price = 1;

        FixedPrice.Sale memory sale = FixedPrice.Sale(
            startId, // uint48 currentId;
            amount, // uint48 finalId;
            address(nft), // address edition;
            price, // uint96 price;
            payable(creator), // address payable saleReceiver;
            uint96(block.timestamp) // uint96 startTime;
        );
        FixedPrice fixedSale = FixedPrice(fixedPriceFactory.createFixedSale(sale));

        nft.grantRole(nft.MINTER_ROLE(), address(fixedSale));

        vm.stopPrank();

        vm.startPrank(buyer);

        fixedSale.buy{value: price * amount}(amount);

        vm.stopPrank();

        // TokenId 0 doesn't exist, the following will revert
        vm.expectRevert("ERC721: invalid token ID");
        nft.ownerOf(0);
    }
}

Recommendation

Change how indices are used during the minting process. Instead of going from sale_.currentId + 1 change it to start in sale_.currentId and change the for condition to x < newId so that the proper amount is minted:

for (uint48 x = sale_.currentId; x < newId; x++) {
    nft.mint(msg.sender, x);
}

Note that this also impacts in the related code that checks for bounds (amount doesn't exceed finalId and the check for last NFT in the series to trigger the end of the sale), that will need to be changed accordingly too.