Escher contest - poirots's results

Lines of code

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

Vulnerability details

Impact

A Dutch Auction is a type of auction in which the auctioneer begins with a high asking price in the case of selling, and lowers it until some participant accepts the price, or it reaches a predetermined reserve price. In this project, an edition is sold through this mechanism by setting the parameters which will rule the auction.

A problem arises when the defined dropPerSecond results in a bigger value than the starting price during or after the auction when calculating the price drop, which results in Arithmetic over/underflow in getPrice(). This in turn prevents the refund and buy functions.

External requirements:

• A badly configured sale.

Proof of Concept

Confirm the behaviour by altering test_LPDA() to:

    function test_LPDA() public {
        // make the lpda sales contract 
        // @audit overwriting original DPS to 1.1, bigger than the starting price
        lpdaSale.dropPerSecond = uint80(uint256(1.1 ether) / 1 days);
        
        sale = LPDA(lpdaSales.createLPDASale(lpdaSale));
        // authorize the lpda sale to mint tokens
        edition.grantRole(edition.MINTER_ROLE(), address(sale));
        //lets buy an NFT
        sale.buy{value: 1 ether}(1);
        assertEq(address(sale).balance, 1 ether);

        vm.warp(block.timestamp + 1 days);
        // @audit getPrice() the culprit, but will also fail in the next call
        assertApproxEqRel(sale.getPrice(), 0.9 ether, lpdaSale.dropPerSecond);

        // buy the rest
        // this will auto end the sale
        sale.buy{value: uint256((0.9 ether + lpdaSale.dropPerSecond) * 9)}(9);

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

        // now lets get a refund
        uint256 bal = address(this).balance;
        sale.refund();

        //assertApproxEqRel(address(this).balance, bal + 0.1 ether, lpdaSale.dropPerSecond);
    }

Tools Used

Foundry

Recommended Mitigation Steps

In order to prevent underflow, when creating an LPDA.Sale check if it's possible with the last requirement:

    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");
        uint96 fullTimeElapsed = sale.endTime - sale.startTime;
        require(sale.dropPerSecond * fullTimeElapsed < sale.startPrice, "DPS MAY UNDERFLOW");

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

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