Escher contest - Madalad's results

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDAFactory.sol#L36 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L117-L125

Vulnerability details

Impact

Concerning the variables within the Sale struct in LPDA, dropPerSecond ought to be such that:

dropPerSecond = (startPrice - finalPrice) / (endTime - startTime)

i.e. price will drop gradually over the period of the sale until it reaches finalPrice once the sale is over, without ever dropping below finalPrice.

However, it is left as a user input in LPDAFactory.createLPDASale(), with the only check being that it is > 0.

Thus, the above equation may not hold, which could lead to price exhibiting unexpected behaviour:

• if dropPerSecond is too high, then by the end of the sale price will drop below finalPrice and remain there for the rest of the sale
• if dropPerSecond is too low, then by the end of the sale price will be above finalPrice, and remain there for the rest of the sale

In each of the above cases, finalPrice is redundant. Such cases would likely mislead users, and does not appear to be intended.

Proof of Concept

function test_DropPerSecondTooHigh() public {
    // setup
    lpdaSale.startPrice = uint80(uint256(1 ether));
    lpdaSale.finalPrice = uint80(uint256(0.1 ether));
    // price should drop to ~0.05 after 1 day
    lpdaSale.dropPerSecond = uint80(uint256(0.95 ether) / 1 days);
    sale = LPDA(lpdaSales.createLPDASale(lpdaSale));
    // authorize the lpda sale to mint tokens
    edition.grantRole(edition.MINTER_ROLE(), address(sale));

    vm.warp(lpdaSale.endTime + 1);

    uint256 price = sale.getPrice(); // ~0.05ETH
    assert(price < lpdaSale.finalPrice);
}

function test_DropPerSecondTooLow() public {
    // setup
    lpdaSale.startPrice = uint80(uint256(1 ether));
    lpdaSale.finalPrice = uint80(uint256(0.1 ether));
    // price should drop to ~0.8 after 1 day
    lpdaSale.dropPerSecond = uint80(uint256(0.2 ether) / 1 days);
    sale = LPDA(lpdaSales.createLPDASale(lpdaSale));
    // authorize the lpda sale to mint tokens
    edition.grantRole(edition.MINTER_ROLE(), address(sale));

    vm.warp(lpdaSale.endTime + 1);

    uint256 price = sale.getPrice(); // ~0.8ETH
    assert(price > lpdaSale.finalPrice);
}

Tools Used

Foundry

Recommended Mitigation Steps

• If the price decrease over the duration of the sale is intended to be consistent, remove the dropPerSecond input and instead compute it within LPDAFactory.createLPDASale() using the equation in the impact section
• If not, alter getPrice() such that if block.timestamp > end then finalPrice is returned instead of the existing calculation

Lines of code

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

Vulnerability details

Impact

In FixedPrice and LPDA, the only way to retreive ether proceeds is the internal function _end().

After the sale begins, this function can only be called within buy() once the final NFT is bought.

If the collection does not ever sell out, the ether will be trapped indefinitely.

Proof of Concept

A creator uses FixedPriceFactory to deploy a FixedPrice contract for their collection, from which they hope to sell 100 NFTs for 1 ether each. After selling 90 of them, interest dies out and no one else wishes to buy any more. FixedPrice holds 90 ether that the creator is unable to access. If no more NFTs are sold, the creator will never be able to access the proceeds of their sales.

function testTrappedEther() public {
    test_Buy();

    vm.warp(365 days);

    vm.expectRevert();
    sale.cancel();

    assertEq(address(sale).balance, 1 ether);
}

Recommended Mitigation Steps

Add new expiryDate variable to the contract, such that after that period the owner is able to call a new function endSale() to end the sale and collect their ether.

contract FixedPrice is ... {

    // ...

    struct Sale {
        // slot 1
        uint48 currentId;
        uint48 finalId;
        address edition;
        // slot 2
        uint96 price;
        address payable saleReceiver;
        // slot 3
        uint96 startTime;
        uint96 expiryDate; // new variable
    }

    //...

    // new function
    function endSale() external onlyOwner {
        require(block.timestamp > sale.endTime, "TOO EARLY");
        _end(sale);
    }
}

contract LPDA is ... {

    //...

    struct Sale {
        // slot 1
        uint48 currentId;
        uint48 finalId;
        address edition;
        // slot 2
        uint80 startPrice;
        uint80 finalPrice;
        uint80 dropPerSecond;
        // slot 3
        uint96 endTime;
        address payable saleReceiver;
        // slot 4
        uint96 startTime;
        uint96 expiryDate; // new variable
    }

    //...

    // new function
    function endSale() external onlyOwner {
        require(block.timestamp > expiryDate, "TOO EARLY");
        _end(sale);
    }
}