Use SafeTransferLib for ETH transfers [GasZipFacet v2.0.1,LibAsset v1…

….0.2,FeeCollector v1.0.1,GasZipPeriphery v1.0.1,LiFiDEXAggregator v1.5.1,LiFuelFeeCollector v1.0.2,Permit2Proxy v1.0.2,Receiver v2.0.3,ReceiverAcrossV3 v1.0.3,ReceiverStargateV2 v1.0.1,RelayerCelerIM v2.0.1,TokenWrapper v1.0.1] (#912) * Improve readability * Small fixes * Add frontrun resistance * Fix max approve * replace .call with safeTransferETH * bump versions * bump versions * Revert unintentional change to RelayFacet * revert changes to GenericSwapFacetV3 * fix tests * add audit report and update log --------- Co-authored-by: Max Klenk <[email protected]>
lifinance · Jan 10, 2025 · 48dcc21 · 48dcc21
1 parent e8bf2bc
commit 48dcc21
Show file tree

Hide file tree

Showing 17 changed files with 187 additions and 112 deletions.
diff --git a/audit/auditLog.json b/audit/auditLog.json
@@ -90,6 +90,13 @@
       "auditorGitHandle": "sujithsomraaj",
       "auditReportPath": "./audit/reports/2025.01.09_ThorSwapFacet(v1.2.1).pdf",
       "auditCommitHash": "5005bf62858a9cb2a7628976ef870e91bb732a75"
+    },
+    "audit20250109_3": {
+      "auditCompletedOn": "09.01.2025",
+      "auditedBy": "Sujith Somraaj (individual security researcher)",
+      "auditorGitHandle": "sujithsomraaj",
+      "auditReportPath": "./audit/reports/2025.01.09_SafeTransferETHUpdate.pdf",
+      "auditCommitHash": "e4f27b0c4f572e5c64ca132dc2e496a3fe6c7c1d"
     }
   },
   "auditedContracts": {
@@ -122,14 +129,25 @@
         "audit20241105"
       ]
     },
+    "FeeCollector": {
+      "1.0.1": [
+        "audit20250109_3"
+      ]
+    },
     "GasZipFacet": {
       "2.0.0": [
         "audit20241107"
+      ],
+      "2.0.1": [
+        "audit20250109_3"
       ]
     },
     "GasZipPeriphery": {
       "1.0.0": [
         "audit20241107"
+      ],
+      "1.0.1": [
+        "audit20250109_3"
       ]
     },
     "IGasZip": {
@@ -140,16 +158,35 @@
     "LibAsset": {
       "1.0.1": [
         "audit20241202"
+      ],
+      "1.0.2": [
+        "audit20250109_3"
       ]
     },
     "LiFiDEXAggregator": {
       "1.5.0": [
         "audit20241203"
+      ],
+      "1.5.1": [
+        "audit20250109_3"
+      ]
+    },
+    "LiFuelFeeCollector": {
+      "1.0.2": [
+        "audit20250109_3"
       ]
     },
     "Permit2Proxy": {
       "1.0.0": [
         "audit20241122"
+      ],
+      "1.0.2": [
+        "audit20250109_3"
+      ]
+    },
+    "Receiver": {
+      "2.0.3": [
+        "audit20250109_3"
       ]
     },
     "ReceiverAcrossV3": {
@@ -158,6 +195,19 @@
       ],
       "1.0.1": [
         "audit20241206"
+      ],
+      "1.0.3": [
+        "audit20250109_3"
+      ]
+    },
+    "ReceiverStargateV2": {
+      "1.0.1": [
+        "audit20250109_3"
+      ]
+    },
+    "RelayerCelerIM": {
+      "2.0.1": [
+        "audit20250109_3"
       ]
     },
     "RelayFacet": {
@@ -175,6 +225,11 @@
         "audit20250109_1"
       ]
     },
+    "TokenWrapper": {
+      "1.0.1": [
+        "audit20250109_3"
+      ]
+    },
     "WithdrawablePeriphery": {
       "1.0.0": [
         "audit20241014"

diff --git a/audit/reports/2025.01.09_SafeTransferETHUpdate.pdf b/audit/reports/2025.01.09_SafeTransferETHUpdate.pdf
diff --git a/src/Facets/GasZipFacet.sol b/src/Facets/GasZipFacet.sol
@@ -14,15 +14,17 @@ import { InvalidCallData, CannotBridgeToSameNetwork, InvalidAmount } from "lifi/
 /// @title GasZipFacet
 /// @author LI.FI (https://li.fi)
 /// @notice Provides functionality to swap ERC20 tokens to native and deposit them to the gas.zip protocol (https://www.gas.zip/)
-/// @custom:version 2.0.0
+/// @custom:version 2.0.1
 contract GasZipFacet is ILiFi, ReentrancyGuard, SwapperV2, Validatable {
     using SafeTransferLib for address;
 
+    uint256 internal constant MAX_CHAINID_LENGTH_ALLOWED = 32;
+
     error OnlyNativeAllowed();
     error TooManyChainIds();
 
     /// State ///
-    address public constant NON_EVM_RECEIVER_IDENTIFIER =
+    address public constant NON_EVM_ADDRESS =
         0x11f111f111f111F111f111f111F111f111f111F1;
     IGasZip public immutable gasZipRouter;
 
@@ -104,7 +106,7 @@ contract GasZipFacet is ILiFi, ReentrancyGuard, SwapperV2, Validatable {
 
         // validate that receiverAddress matches with bridgeData in case of EVM target chain
         if (
-            _bridgeData.receiver != NON_EVM_RECEIVER_IDENTIFIER &&
+            _bridgeData.receiver != NON_EVM_ADDRESS &&
             _gasZipData.receiverAddress !=
             bytes32(uint256(uint160(_bridgeData.receiver)))
         ) revert InvalidCallData();
@@ -130,7 +132,7 @@ contract GasZipFacet is ILiFi, ReentrancyGuard, SwapperV2, Validatable {
     ) external pure returns (uint256 destinationChains) {
         uint256 length = _chainIds.length;
 
-        if (length > 32) revert TooManyChainIds();
+        if (length > MAX_CHAINID_LENGTH_ALLOWED) revert TooManyChainIds();
 
         for (uint256 i; i < length; ++i) {
             // Shift destinationChains left by 8 bits and add the next chainID

diff --git a/src/Libraries/LibAsset.sol b/src/Libraries/LibAsset.sol
@@ -6,7 +6,7 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { LibSwap } from "./LibSwap.sol";
 
 /// @title LibAsset
-/// @custom:version 1.0.1
+/// @custom:version 1.0.2
 /// @notice This library contains helpers for dealing with onchain transfers
 ///         of assets, including accounting for the native asset `assetId`
 ///         conventions and any noncompliant ERC20 transfers
@@ -67,8 +67,7 @@ library LibAsset {
         }
 
         if (assetId.allowance(address(this), spender) < amount) {
-            SafeERC20.safeApprove(IERC20(assetId), spender, 0);
-            SafeERC20.safeApprove(IERC20(assetId), spender, MAX_UINT);
+            SafeERC20.forceApprove(IERC20(assetId), spender, MAX_UINT);
         }
     }
 

diff --git a/src/Periphery/FeeCollector.sol b/src/Periphery/FeeCollector.sol
@@ -3,11 +3,12 @@ pragma solidity ^0.8.17;
 
 import { LibAsset } from "../Libraries/LibAsset.sol";
 import { TransferrableOwnership } from "../Helpers/TransferrableOwnership.sol";
+import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 
 /// @title Fee Collector
 /// @author LI.FI (https://li.fi)
 /// @notice Provides functionality for collecting integrator fees
-/// @custom:version 1.0.0
+/// @custom:version 1.0.1
 contract FeeCollector is TransferrableOwnership {
     /// State ///
 
@@ -84,12 +85,7 @@ contract FeeCollector is TransferrableOwnership {
         // Prevent extra native token from being locked in the contract
         if (remaining > 0) {
             // solhint-disable-next-line avoid-low-level-calls
-            (bool success, ) = payable(msg.sender).call{ value: remaining }(
-                ""
-            );
-            if (!success) {
-                revert TransferFailure();
-            }
+            SafeTransferLib.safeTransferETH(msg.sender, remaining);
         }
         emit FeesCollected(
             LibAsset.NULL_ADDRESS,

diff --git a/src/Periphery/GasZipPeriphery.sol b/src/Periphery/GasZipPeriphery.sol
@@ -6,29 +6,21 @@ import { IGasZip } from "../Interfaces/IGasZip.sol";
 import { LibSwap } from "../Libraries/LibSwap.sol";
 import { LibAsset, IERC20 } from "../Libraries/LibAsset.sol";
 import { LibUtil } from "../Libraries/LibUtil.sol";
-import { ReentrancyGuard } from "../Helpers/ReentrancyGuard.sol";
-import { SwapperV2 } from "../Helpers/SwapperV2.sol";
 import { WithdrawablePeriphery } from "../Helpers/WithdrawablePeriphery.sol";
-import { Validatable } from "../Helpers/Validatable.sol";
 import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 import { InvalidCallData } from "../Errors/GenericErrors.sol";
 
 /// @title GasZipPeriphery
 /// @author LI.FI (https://li.fi)
 /// @notice Provides functionality to swap ERC20 tokens to use the gas.zip protocol as a pre-bridge step (https://www.gas.zip/)
-/// @custom:version 1.0.0
-contract GasZipPeriphery is
-    ILiFi,
-    ReentrancyGuard,
-    SwapperV2,
-    Validatable,
-    WithdrawablePeriphery
-{
+/// @custom:version 1.0.1
+contract GasZipPeriphery is ILiFi, WithdrawablePeriphery {
     using SafeTransferLib for address;
 
     /// State ///
     IGasZip public immutable gasZipRouter;
     address public immutable liFiDEXAggregator;
+    uint256 internal constant MAX_CHAINID_LENGTH_ALLOWED = 32;
 
     /// Errors ///
     error TooManyChainIds();
@@ -59,7 +51,7 @@ contract GasZipPeriphery is
         LibAsset.maxApproveERC20(
             IERC20(_swapData.sendingAssetId),
             liFiDEXAggregator,
-            type(uint256).max
+            _swapData.fromAmount
         );
 
         // execute swap using LiFiDEXAggregator
@@ -110,7 +102,7 @@ contract GasZipPeriphery is
     ) external pure returns (uint256 destinationChains) {
         uint256 length = _chainIds.length;
 
-        if (length > 32) revert TooManyChainIds();
+        if (length > MAX_CHAINID_LENGTH_ALLOWED) revert TooManyChainIds();
 
         for (uint256 i; i < length; ++i) {
             // Shift destinationChains left by 8 bits and add the next chainID

diff --git a/src/Periphery/LiFiDEXAggregator.sol b/src/Periphery/LiFiDEXAggregator.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.17;
 
 import { SafeERC20, IERC20, IERC20Permit } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
+import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 
 address constant NATIVE_ADDRESS = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
 address constant IMPOSSIBLE_POOL_ADDRESS = 0x0000000000000000000000000000000000000001;
@@ -22,7 +23,7 @@ uint160 constant MAX_SQRT_RATIO = 1461446703485210103287273052203988822378723970
 /// @title LiFi DEX Aggregator
 /// @author Ilya Lyalin (contract copied from: https://github.com/sushiswap/sushiswap/blob/c8c80dec821003eb72eb77c7e0446ddde8ca9e1e/protocols/route-processor/contracts/RouteProcessor4.sol)
 /// @notice Processes calldata to swap using various DEXs
-/// @custom:version 1.5.0
+/// @custom:version 1.5.1
 contract LiFiDEXAggregator is Ownable {
     using SafeERC20 for IERC20;
     using Approve for IERC20;
@@ -130,14 +131,7 @@ contract LiFiDEXAggregator is Ownable {
         address to,
         bytes memory route
     ) external payable lock returns (uint256 amountOut) {
-        (bool success, bytes memory returnBytes) = transferValueTo.call{
-            value: amountValueTransfer
-        }("");
-        if (!success) {
-            assembly {
-                revert(add(32, returnBytes), mload(returnBytes))
-            }
-        }
+        SafeTransferLib.safeTransferETH(transferValueTo, amountValueTransfer);
         return
             processRouteInternal(
                 tokenIn,
@@ -370,11 +364,7 @@ contract LiFiDEXAggregator is Ownable {
                     );
                 IWETH(tokenIn).withdraw(amountIn);
             }
-            (bool success, ) = payable(to).call{ value: amountIn }("");
-            require(
-                success,
-                "RouteProcessor.wrapNative: Native token transfer failed"
-            );
+            SafeTransferLib.safeTransferETH(to, amountIn);
         }
     }
 
@@ -750,11 +740,7 @@ contract LiFiDEXAggregator is Ownable {
 
         if (to != address(this)) {
             if (tokenOut == NATIVE_ADDRESS) {
-                (bool success, ) = payable(to).call{ value: amountOut }("");
-                require(
-                    success,
-                    "RouteProcessor.swapCurve: Native token transfer failed"
-                );
+                SafeTransferLib.safeTransferETH(to, amountOut);
             } else {
                 IERC20(tokenOut).safeTransfer(to, amountOut);
             }

diff --git a/src/Periphery/LiFuelFeeCollector.sol b/src/Periphery/LiFuelFeeCollector.sol
@@ -3,11 +3,12 @@ pragma solidity ^0.8.17;
 
 import { LibAsset } from "../Libraries/LibAsset.sol";
 import { TransferrableOwnership } from "../Helpers/TransferrableOwnership.sol";
+import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol";
 
 /// @title LiFuelFeeCollector
 /// @author LI.FI (https://li.fi)
 /// @notice Provides functionality for collecting fees for LiFuel
-/// @custom:version 1.0.1
+/// @custom:version 1.0.2
 contract LiFuelFeeCollector is TransferrableOwnership {
     /// Errors ///
     error TransferFailure();
@@ -65,10 +66,7 @@ contract LiFuelFeeCollector is TransferrableOwnership {
         );
         uint256 amountMinusFees = msg.value - feeAmount;
         if (amountMinusFees > 0) {
-            (bool success, ) = msg.sender.call{ value: amountMinusFees }("");
-            if (!success) {
-                revert TransferFailure();
-            }
+            SafeTransferLib.safeTransferETH(msg.sender, amountMinusFees);
         }
     }