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

0 stars 0 forks source link

LPDA sales can potentially have buying function reverting indefinately due to negative price #437

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

While i'm assuming there are checks for this on UI or off-chain, this wasn't mentioned in the docs or the comments, so its worth flagging and possibly implementating the mitigation step as a precaution since it is also cheap on gas.

The price can theoritically drop below the sale.startprice which can lead to getPrice() function call to always revert which will in turn always revert a buy function call under these conditions.

More specifically on line 125 of the code above:

priceDropPerSec * timeElapased can be larger than start price.

There is no steps taken in code to check or mitigate it and hence this can be potential source of DoS.

Proof of Concept

The below test using forge fails due to revert

       function test_LPDABug() public {

        lpdaSale = LPDA.Sale({
            currentId: uint48(0),
            finalId: uint48(10),
            edition: address(edition),
            startPrice: uint80(uint256(1 ether)),
            finalPrice: uint80(uint256(0.1 ether)),
            dropPerSecond: uint80(uint256(1.1 ether) / 1 days),
            startTime: uint96(block.timestamp),
            saleReceiver: payable(address(69)),
            endTime: uint96(block.timestamp + 1 days)
        });
        // make the lpda sales contract
        sale = LPDA(lpdaSales.createLPDASale(lpdaSale));
        // authorize the lpda sale to mint tokens
        edition.grantRole(edition.MINTER_ROLE(), address(sale));

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

        sale.getPrice();
    }

Tools Used

VSCode Forge for test

Recommended Mitigation Steps

Updating the return statement of line 125 of LDPA.sol to something like the below to return final price incase the price drop ends up being bigger than start price

uint256 dropAmount = temp.dropPerSecond * timeElapsed;
return dropAmount > temp.startPrice ?  temp.final : Pricetemp.startPrice - dropAmount;
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)