code-423n4 / 2022-12-escher-findings

0 stars 0 forks source link

underflow in the `getPrice()` function can block the `buy` and `refund` in the `LPDA` sale #534

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In the LPDA sale the price decrease in values after each second, and when creating the sale the value of the lowest price possible is not checked, so the price could go below zero at a given timestamp which will lead to an underflow in the getPrice() function which will block the buy and refund operations forever.

Proof of Concept

The issue occurs in the getPrice function below :

File: src/minters/LPDA.sol Line 117-125

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);
}

As you can see the current price is calculated using the formula :

temp.startPrice - (temp.dropPerSecond * timeElapsed);

And when creating the LPDA sale only the non zero checks are performed on the startPrice and dropPerSecond :

require(sale.startPrice > 0, "INVALID START PRICE");
require(sale.dropPerSecond > 0, "INVALID DROP PER SECOND");

So a user can set any value for those two variable apart from 0, and thus he can set a big value for dropPerSecond or a small value for startPrice or the sale period (endTime - startTime) could be too big, this can cause an underflow in the calculated price at a certain timestamp or the lowest price could be below zero, which will lead the getPrice() function to revert for all the calls.

Because both the buy and refund functions call the getPrice function, if an underflow occurs the later will revert and thus the buy (or refund) function will revert.

In the end the impact of this is that users will not be able to buy or to get refund for their funds.

Tools Used

Manual review

Recommended Mitigation Steps

To solve this issue the lowest price possible for a sale should be checked when creating the LPDA sale and its value should be non zero, the createLPDASale function should be modified as follow :

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");

    /**
        Calculate the lowest price and check that its non zero
    */
    uint256 lowestPrice = sale.startPrice - (sale.dropPerSecond * (sale.endTime - sale.startTime));
    require(lowestPrice > 0, "INVALID LOWEST PRICE");

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

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

By doing so you ensure that the getPrice function will never underflow, as all the prices are above or equal to the lowest price they all are above zero.

c4-judge commented 1 year ago

berndartmueller marked the issue as duplicate of #392

c4-judge commented 1 year ago

berndartmueller marked the issue as satisfactory

c4-judge commented 1 year ago

berndartmueller changed the severity to 3 (High Risk)