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

0 stars 0 forks source link

`LPDA` refund logic is broken, meaning buyers always get lowest price sale #541

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#L101

Vulnerability details

Impact

The protocol intends the LPDA to refunds buyers with the difference between the price they paid and the last sale price

Once the sale has ended, the users must call refund to get their Ether refunds based on their purchase price and lowest sale price
99:     function refund() public {
100:         Receipt memory r = receipts[msg.sender];
101:         uint80 price = uint80(getPrice()) * r.amount; //@audit - not lowest sale, lowest price
102:         uint80 owed = r.balance - price;
103:         require(owed > 0, "NOTHING TO REFUND");
104:         receipts[msg.sender].balance = price;
105:         payable(msg.sender).transfer(owed);
106:     }

But if the buyer waits for the endTime, the price returned will not be the lowest sale price, but instead the lowest price that would be at the end of the auction

117:     function getPrice() public view returns (uint256) {
118:         Sale memory temp = sale;
119:         (uint256 start, uint256 end) = (temp.startTime, temp.endTime);
120:         if (block.timestamp < start) return type(uint256).max;
121:         if (temp.currentId == temp.finalId) return temp.finalPrice;
122: 
123:         uint256 timeElapsed = end > block.timestamp ? block.timestamp - start : end - start;
124:         return temp.startPrice - (temp.dropPerSecond * timeElapsed); //@audit - lowest price not taking sales into account
125:     }

This means buyers can simply call refund after the end, which will mean they will have only paid the lowest auction amount possible, regardless of the last auction price.

Impact

This essentially breaks the LPDA, because all buyers will be able to buy at the lowest possible amount. Because it allows users to game the auction, I consider this a High severity issue.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using a storage variable in buy to track the timestamp of the latest sale, and use it in getPrice to return the correct amount in refund

Minh-Trng commented 1 year ago

wrong, there is an early return if sale has ended due to reaching finalId Mint: if (temp.currentId == temp.finalId) return temp.finalPrice;

If the final mint has not been reached the price moves lower with each second as intended

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid