Escher contest - Franfran's results

Lines of code

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

Vulnerability details

Impact

In the LPDA contract, there is a function called getPrice() which returns the price of one token by taking into account the drop in price per second of the Dutch auction. It basically calculates how much time was elapsed since the start of the sale, to return the price of the asset at the time that the function is called. The issue is that the parameters are not sanitized when set, and the return value can revert for some values of these parameters.

See the return value of this function: https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L124

    function getPrice() public view returns (uint256) {
        Sale memory temp = sale;
        (uint256 start, uint256 end) = (temp.startTime, temp.endTime);
        if (block.timestamp < start) return type(uint256).max;
        if (temp.currentId == temp.finalId) return temp.finalPrice;

        uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start;
        return temp.startPrice - (temp.dropPerSecond * timeElapsed);
    }

This means that, with the built-in overflow checked of solidity +0.8.0, if temp.dropPerSecond * timeElapsed is any greater than temp.startPrice, this function is going to revert because of underflow.

Any call to this function will revert, and these functions are buy() and refund().

If the dropPerSecond, startTime, and endTime variables are not set sensibly from the start, then the contract is going to enter into a frozen state, reverting any buy() of refund(), so any ETH that was sent to the contract would be forever stuck.

Proof of Concept

This POC demonstrates what kind of values would make the contract go into this frozen state:

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

import "forge-std/Test.sol";
import {EscherTest} from "../utils/EscherTest.sol";
import {LPDAFactory, LPDA} from "src/minters/LPDAFactory.sol";

contract LPDAAudit is EscherTest {
    LPDAFactory public lpdaSales;
    LPDA public sale;

    function setUp() public virtual override {
        super.setUp();
        lpdaSales = new LPDAFactory();
    }

    function test_frozen_LPDA() public {
        // make the lpda sales contract
        sale = LPDA(
            lpdaSales.createLPDASale(
                LPDA.Sale({
                    currentId: uint48(0),
                    finalId: uint48(10),
                    edition: address(edition),
                    startPrice: uint80(uint256(1 ether)),
                    finalPrice: uint80(uint256(0.1 ether)),
                    // The dropPerSecond is too big, and will make the `getPrice()` revert at some point
                    dropPerSecond: uint80(uint256(2 ether) / 1 days),
                    startTime: uint96(block.timestamp),
                    saleReceiver: payable(address(69)),
                    endTime: uint96(block.timestamp + 1 days)
                })
            )
        );

        vm.warp(block.timestamp + 2 days);

        (
            ,
            ,
            ,
            uint80 startPrice,
            uint80 finalPrice,
            uint80 dropPerSecond,
            uint96 endTime,
            ,
            uint96 startTime
        ) = sale.sale();

        assertGt(dropPerSecond * (endTime - startTime), startPrice);

        vm.expectRevert();
        // getPrice() now reverts forever
        sale.getPrice();

        // refund() as well
        vm.expectRevert();
        sale.refund();
    }
}

Tools Used

Manual inspection

Recommended Mitigation Steps

In the LPDAFactory, make sure that the dropPerSecond matches the startTime and endTime.

    function createLPDASale(LPDA.Sale calldata sale) external returns (address clone) {
        require(IEscher721(sale.edition).hasRole(bytes32(0x00), msg.sender), "NOT AUTHORIZED");
        require(sale.saleReceiver != address(0), "INVALID SALE RECEIVER");
        require(sale.startTime >= block.timestamp, "INVALID START TIME");
        require(sale.endTime > sale.startTime, "INVALID END TIME");
        require(sale.finalId > sale.currentId, "INVALID FINAL ID");
        require(sale.startPrice > 0, "INVALID START PRICE");
        require(sale.dropPerSecond > 0, "INVALID DROP PER SECOND");

        + require(sale.dropPerSecond * (sale.endTime - sale.startTime) >= sale.startPrice, "DROP IS TOO STEEP");

        clone = implementation.clone();
        LPDA(clone).initialize(sale);

        emit NewLPDAContract(msg.sender, sale.edition, clone, sale);
    }