Ether can get lock on functions `takeMultipleOneOrders` and `takeOrders`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L326 https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L362

Vulnerability details

Impact

Ether send by the user cang gets locks when taking order/s

Proof of Concept

If Bob use function takeMultipleOneOrders or takeOrders to buy and sends more ETH that it supposes to remaing ETH will be lost, also if the seller is selling for other token and sends ETH by mistake ETH will be lost.

Tools Used

Manual revision

Recommended Mitigation Steps

There are two things that you could do, make a exact match require for the ether send or give user remaing ETH back.

diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol
index 3639554..59c5f0f 100644
--- a/contracts/core/InfinityExchange.sol
+++ b/contracts/core/InfinityExchange.sol
@@ -323,7 +323,9 @@ contract InfinityExchange is ReentrancyGuard, Ownable {
     // check to ensure that for ETH orders, enough ETH is sent
     // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent
     if (isMakerSeller && currency == address(0)) {
-      require(msg.value >= totalPrice, 'invalid total price');
+      require(msg.value == totalPrice, 'invalid total price');
+    } else {
+      require(msg.value == 0, 'invalid total price');
     }
   }

@@ -359,7 +361,9 @@ contract InfinityExchange is ReentrancyGuard, Ownable {
     // check to ensure that for ETH orders, enough ETH is sent
     // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent
     if (isMakerSeller && currency == address(0)) {
-      require(msg.value >= totalPrice, 'invalid total price');
+      require(msg.value == totalPrice, 'invalid total price');
+    } else {
+      require(msg.value == 0, 'invalid total price');
     }
   }

Another alternative could be sending back to the user the reimaing ETH

diff --git a/contracts/core/InfinityExchange.sol b/contracts/core/InfinityExchange.sol
index 3639554..94d5a30 100644
--- a/contracts/core/InfinityExchange.sol
+++ b/contracts/core/InfinityExchange.sol
@@ -324,6 +324,15 @@ contract InfinityExchange is ReentrancyGuard, Ownable {
     // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent
     if (isMakerSeller && currency == address(0)) {
       require(msg.value >= totalPrice, 'invalid total price');
+      if (msg.value > totalPrice) {
+        // transfer remaining amount to buyer
+        (bool sent, ) = msg.sender.call{value: msg.value - totalPrice}('');
+        require(sent, 'failed to send ether to seller');
+      }
+    } else if (msg.value > 0 ) {
+      // transfer remaining amount to buyer
+      (bool sent, ) = msg.sender.call{value: msg.value}('');
+      require(sent, 'failed to send ether to seller');
     }
   }

@@ -360,6 +369,15 @@ contract InfinityExchange is ReentrancyGuard, Ownable {
     // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent
     if (isMakerSeller && currency == address(0)) {
       require(msg.value >= totalPrice, 'invalid total price');
+      if (msg.value > totalPrice) {
+        // transfer remaining amount to buyer
+        (bool sent, ) = msg.sender.call{value: msg.value - totalPrice}('');
+        require(sent, 'failed to send ether to seller');
+      }
+    } else if (msg.value > 0 ) {
+      // transfer remaining amount to buyer
+      (bool sent, ) = msg.sender.call{value: msg.value}('');
+      require(sent, 'failed to send ether to seller');
     }
   }

[KenzoAgada]

Duplicate of #244

[HardlyDifficult]

Dupe of https://github.com/code-423n4/2022-06-infinity-findings/issues/244