Escher contest - chaduke's results

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. The refund() function will call the getPrice() to get the price. However, when sale.startPrice - sale.dropPerSecond *(sale.startTime-sale.endTime) < 0, it is possible that getPrice() might revert due to underflow. As a result, the refund will revert. The user can not get refund even though he/she deserves a full refund.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. The failure of refund is caused by the lack of check of the condition sale.startPrice - sale.dropPerSecond *(sale.startTime-sale.endTime) >=0. As a result, after some time point, the getPrice() function will always revert at the last line due to underflow. A user will not be able to get refund even though he/she deserves to have a full refund.

One can check this condition at the factory contract to ensure getPrice() at (https://github.com/code-423n4/2022-12-escher/blob/5d8be6aa0e8634fdb2f328b99076b0d05fefab73/src/minters/LPDA.sol#L117) never underflows, otherwise, refunding will be unfair: some can get refund, some cannot.

Consider the special scenario that a seller likes to sell all NFTS within one 24 hrs otherwise, after that, the price will become zero and users can get full refund. However, such a plan will fail if we allow sale.startPrice - sale.dropPerSecond *(sale.startTime-sale.endTime) to be a negative value. In particular, a user A might get a refund when the price is still above zero, but another user cannot get a refund because when the function getPrice() is called, there is a underflow at the last line.

Tools Used

Remix

Recommended Mitigation Steps

There are two solutions depending on what scenarios we need to support (including the one introduced above):

1. change the last line of the getPrice() function such that when the price becomes negative, we return zero as the price to avoid underflow. This will support full refund scenarios.
2. check at the factory to make sure sale.startPrice - sale.dropPerSecond *(sale.startTime-sale.endTime) >= 0 at the beginning of function: https://github.com/code-423n4/2022-12-escher/blob/5d8be6aa0e8634fdb2f328b99076b0d05fefab73/src/minters/LPDAFactory.sol#L29 This will ensure getPrice() has no underflow issue.

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. Solidity’s transfer() and send() use a hardcoded gas amount which might change in the future. These methods should be avoided. Use .call.value(...)("") instead. See Consensys's recommendation of not using them https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/code-423n4/2022-12-escher/blob/5d8be6aa0e8634fdb2f328b99076b0d05fefab73/src/minters/LPDA.sol#L105

Tools Used

Remix

Recommended Mitigation Steps

use .call.value(...)("") instead to send eth.