diff --git a/audit/auditLog.json b/audit/auditLog.json index 0785335186..ea2931ad42 100644 --- a/audit/auditLog.json +++ b/audit/auditLog.json @@ -580,6 +580,13 @@ "auditorGitHandle": "sujithsomraaj", "auditReportPath": "./audit/reports/2026.04.30_DeBridgeDlnFacet(v1.1.0).pdf", "auditCommitHash": "3b8511fce5f2e9b12a50d77f147536c641303cae" + }, + "audit20260519": { + "auditCompletedOn": "19.05.2026", + "auditedBy": "Sujith Somraaj (individual security researcher)", + "auditorGitHandle": "sujithsomraaj", + "auditReportPath": "./audit/reports/2026.05.19_GenericSwapFacetV3(v2.0.0).pdf", + "auditCommitHash": "85d2e87d0e46c785ffb63d52fa4b0cc6bf995a85" } }, "auditedContracts": { @@ -705,7 +712,8 @@ }, "GenericSwapFacetV3": { "1.0.1": ["audit20250508"], - "1.0.2": ["audit20250610"] + "1.0.2": ["audit20250610"], + "2.0.0": ["audit20260519"] }, "GardenFacet": { "1.0.0": ["audit20250919"] diff --git a/audit/reports/2026.05.19_GenericSwapFacetV3(v2.0.0).pdf b/audit/reports/2026.05.19_GenericSwapFacetV3(v2.0.0).pdf new file mode 100644 index 0000000000..e077bb17b1 Binary files /dev/null and b/audit/reports/2026.05.19_GenericSwapFacetV3(v2.0.0).pdf differ diff --git a/src/Facets/GenericSwapFacetV3.sol b/src/Facets/GenericSwapFacetV3.sol index 896a3a7b61..e8fd5b72a4 100644 --- a/src/Facets/GenericSwapFacetV3.sol +++ b/src/Facets/GenericSwapFacetV3.sol @@ -6,20 +6,23 @@ import { LibUtil } from "../Libraries/LibUtil.sol"; import { LibSwap } from "../Libraries/LibSwap.sol"; import { LibAllowList } from "../Libraries/LibAllowList.sol"; import { LibAsset } from "../Libraries/LibAsset.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ContractCallNotAllowed, CumulativeSlippageTooHigh, NativeAssetTransferFailed } from "../Errors/GenericErrors.sol"; -import { ERC20, SafeTransferLib } from "solmate/utils/SafeTransferLib.sol"; /// @title GenericSwapFacetV3 /// @author LI.FI (https://li.fi) /// @notice Provides gas-optimized functionality for fee collection and for swapping through any whitelisted DEX /// @dev Can only execute calldata for whitelisted function selectors -/// @custom:version 1.0.2 +/// @custom:version 2.0.0 contract GenericSwapFacetV3 is ILiFi { - using SafeTransferLib for ERC20; - /// Storage address public immutable NATIVE_ADDRESS; + /// @dev Sentinel selector used to whitelist contracts that are approveTo-only targets + /// (i.e. not callable directly, but need allowance set against them). + /// See LibAllowList "Special ApproveTo-Only Selector" documentation. + bytes4 private constant APPROVE_TO_ONLY_SELECTOR = 0xffffffff; + /// Constructor /// @param _nativeAddress the address of the native token for this network constructor(address _nativeAddress) { @@ -51,7 +54,7 @@ contract GenericSwapFacetV3 is ILiFi { address sendingAssetId = _swapData.sendingAssetId; // get contract's balance (which will be sent in full to user) - uint256 amountReceived = ERC20(receivingAssetId).balanceOf( + uint256 amountReceived = IERC20(receivingAssetId).balanceOf( address(this) ); @@ -60,7 +63,7 @@ contract GenericSwapFacetV3 is ILiFi { revert CumulativeSlippageTooHigh(_minAmountOut, amountReceived); // transfer funds to receiver - ERC20(receivingAssetId).safeTransfer(_receiver, amountReceived); + LibAsset.transferERC20(receivingAssetId, _receiver, amountReceived); // emit events (both required for tracking) uint256 fromAmount = _swapData.fromAmount; @@ -156,10 +159,12 @@ contract GenericSwapFacetV3 is ILiFi { LibSwap.SwapData calldata _swapData ) external payable { address callTo = _swapData.callTo; - // ensure that contract (callTo) and function selector are whitelisted + // ensure that contract (callTo) and function selector are whitelisted as a pair if ( - !(LibAllowList.contractIsAllowed(callTo) && - LibAllowList.selectorIsAllowed(bytes4(_swapData.callData[:4]))) + !LibAllowList.contractSelectorIsAllowed( + callTo, + bytes4(_swapData.callData[:4]) + ) ) revert ContractCallNotAllowed(); // execute swap @@ -175,7 +180,7 @@ contract GenericSwapFacetV3 is ILiFi { // get contract's balance (which will be sent in full to user) address receivingAssetId = _swapData.receivingAssetId; - uint256 amountReceived = ERC20(receivingAssetId).balanceOf( + uint256 amountReceived = IERC20(receivingAssetId).balanceOf( address(this) ); @@ -184,7 +189,7 @@ contract GenericSwapFacetV3 is ILiFi { revert CumulativeSlippageTooHigh(_minAmountOut, amountReceived); // transfer funds to receiver - ERC20(receivingAssetId).safeTransfer(_receiver, amountReceived); + LibAsset.transferERC20(receivingAssetId, _receiver, amountReceived); // emit events (both required for tracking) uint256 fromAmount = _swapData.fromAmount; @@ -306,7 +311,8 @@ contract GenericSwapFacetV3 is ILiFi { if (currentSwap.requiresDeposit) { // we will not check msg.value as tx will fail anyway if not enough value available // thus we only deposit ERC20 tokens here - ERC20(currentSwap.sendingAssetId).safeTransferFrom( + LibAsset.transferFromERC20( + currentSwap.sendingAssetId, msg.sender, address(this), currentSwap.fromAmount @@ -322,38 +328,47 @@ contract GenericSwapFacetV3 is ILiFi { LibSwap.SwapData calldata _swapData, address _receiver ) private { - ERC20 sendingAsset = ERC20(_swapData.sendingAssetId); + address sendingAssetId = _swapData.sendingAssetId; uint256 fromAmount = _swapData.fromAmount; // deposit funds - sendingAsset.safeTransferFrom(msg.sender, address(this), fromAmount); + LibAsset.transferFromERC20( + sendingAssetId, + msg.sender, + address(this), + fromAmount + ); - // ensure that contract (callTo) and function selector are whitelisted + // ensure that contract (callTo) and function selector are whitelisted as a pair address callTo = _swapData.callTo; address approveTo = _swapData.approveTo; bytes calldata callData = _swapData.callData; if ( - !(LibAllowList.contractIsAllowed(callTo) && - LibAllowList.selectorIsAllowed(bytes4(callData[:4]))) + !LibAllowList.contractSelectorIsAllowed( + callTo, + bytes4(callData[:4]) + ) ) revert ContractCallNotAllowed(); - // ensure that approveTo address is also whitelisted if it differs from callTo - if (approveTo != callTo && !LibAllowList.contractIsAllowed(approveTo)) - revert ContractCallNotAllowed(); + // ensure that approveTo is whitelisted as an approveTo-only target if it differs from callTo + if ( + approveTo != callTo && + !LibAllowList.contractSelectorIsAllowed( + approveTo, + APPROVE_TO_ONLY_SELECTOR + ) + ) revert ContractCallNotAllowed(); - // check if the current allowance is sufficient - uint256 currentAllowance = sendingAsset.allowance( - address(this), - approveTo + // set max approval if current allowance is insufficient + // uses solady's safeApproveWithRetry under the hood which first + // attempts a direct approve(spender, max) and only resets to zero + // before retrying if the first call fails (handles USDT-style tokens + // that require allowance to be zero before changing) + LibAsset.maxApproveERC20( + IERC20(sendingAssetId), + approveTo, + fromAmount ); - // check if existing allowance is sufficient - if (currentAllowance < fromAmount) { - // check if is non-zero, set to 0 if not - if (currentAllowance != 0) sendingAsset.safeApprove(approveTo, 0); - // set allowance to uint max to avoid future approvals - sendingAsset.safeApprove(approveTo, type(uint256).max); - } - // execute swap // solhint-disable-next-line avoid-low-level-calls (bool success, bytes memory res) = callTo.call(callData); @@ -361,7 +376,7 @@ contract GenericSwapFacetV3 is ILiFi { LibUtil.revertWith(res); } - _returnPositiveSlippageERC20(sendingAsset, _receiver); + _returnPositiveSlippageERC20(sendingAssetId, _receiver); } // @dev: this function will not work with swapData that has multiple swaps with the same sendingAssetId @@ -375,35 +390,35 @@ contract GenericSwapFacetV3 is ILiFi { ) private { // initialize variables before loop to save gas uint256 numOfSwaps = _swapData.length; - ERC20 sendingAsset; address sendingAssetId; address receivingAssetId; LibSwap.SwapData calldata currentSwap; bool success; bytes memory returnData; - uint256 currentAllowance; // go through all swaps for (uint256 i = 0; i < numOfSwaps; ) { currentSwap = _swapData[i]; sendingAssetId = currentSwap.sendingAssetId; - sendingAsset = ERC20(currentSwap.sendingAssetId); receivingAssetId = currentSwap.receivingAssetId; - // check if callTo address is whitelisted + // check if callTo and selector are whitelisted as a pair if ( - !LibAllowList.contractIsAllowed(currentSwap.callTo) || - !LibAllowList.selectorIsAllowed( + !LibAllowList.contractSelectorIsAllowed( + currentSwap.callTo, bytes4(currentSwap.callData[:4]) ) ) { revert ContractCallNotAllowed(); } - // if approveTo address is different to callTo, check if it's whitelisted, too + // if approveTo differs from callTo, it must be whitelisted as an approveTo-only target if ( currentSwap.approveTo != currentSwap.callTo && - !LibAllowList.contractIsAllowed(currentSwap.approveTo) + !LibAllowList.contractSelectorIsAllowed( + currentSwap.approveTo, + APPROVE_TO_ONLY_SELECTOR + ) ) { revert ContractCallNotAllowed(); } @@ -425,18 +440,16 @@ contract GenericSwapFacetV3 is ILiFi { _returnPositiveSlippageNative(_receiver); } else { // ERC20 - // check if the current allowance is sufficient - currentAllowance = sendingAsset.allowance( - address(this), - currentSwap.approveTo + // set max approval if current allowance is insufficient + // uses solady's safeApproveWithRetry under the hood which first + // attempts a direct approve(spender, max) and only resets to zero + // before retrying if the first call fails (handles USDT-style tokens + // that require allowance to be zero before changing) + LibAsset.maxApproveERC20( + IERC20(sendingAssetId), + currentSwap.approveTo, + currentSwap.fromAmount ); - if (currentAllowance < currentSwap.fromAmount) { - sendingAsset.safeApprove(currentSwap.approveTo, 0); - sendingAsset.safeApprove( - currentSwap.approveTo, - type(uint256).max - ); - } // execute the swap (success, returnData) = currentSwap.callTo.call( @@ -450,7 +463,7 @@ contract GenericSwapFacetV3 is ILiFi { // but only for swaps, not for fee collections // (otherwise the whole amount would be returned before the actual swap) if (sendingAssetId != receivingAssetId) - _returnPositiveSlippageERC20(sendingAsset, _receiver); + _returnPositiveSlippageERC20(sendingAssetId, _receiver); } // emit AssetSwapped event @@ -467,7 +480,7 @@ contract GenericSwapFacetV3 is ILiFi { currentSwap.fromAmount, LibAsset.isNativeAsset(receivingAssetId) ? address(this).balance - : ERC20(receivingAssetId).balanceOf(address(this)), + : IERC20(receivingAssetId).balanceOf(address(this)), block.timestamp ); @@ -488,14 +501,14 @@ contract GenericSwapFacetV3 is ILiFi { // determine the end result of the swap address finalAssetId = _swapData[_swapData.length - 1] .receivingAssetId; - uint256 amountReceived = ERC20(finalAssetId).balanceOf(address(this)); + uint256 amountReceived = IERC20(finalAssetId).balanceOf(address(this)); // make sure minAmountOut was received if (amountReceived < _minAmountOut) revert CumulativeSlippageTooHigh(_minAmountOut, amountReceived); // transfer to receiver - ERC20(finalAssetId).safeTransfer(_receiver, amountReceived); + LibAsset.transferERC20(finalAssetId, _receiver, amountReceived); // emit event emit ILiFi.LiFiGenericSwapCompleted( @@ -546,12 +559,12 @@ contract GenericSwapFacetV3 is ILiFi { // returns any unused 'sendingAsset' tokens (=> positive slippage) to the receiver address function _returnPositiveSlippageERC20( - ERC20 sendingAsset, + address sendingAssetId, address receiver ) private { // if a balance exists in sendingAsset, it must be positive slippage - if (address(sendingAsset) != NATIVE_ADDRESS) { - uint256 sendingAssetBalance = sendingAsset.balanceOf( + if (sendingAssetId != NATIVE_ADDRESS) { + uint256 sendingAssetBalance = IERC20(sendingAssetId).balanceOf( address(this) ); @@ -560,7 +573,11 @@ contract GenericSwapFacetV3 is ILiFi { // but this 1 wei is not transferable, so the tx reverts. We accept that 1 wei dust gets stuck in the contract // with every tx as this does not represent a significant USD value in any relevant token. if (sendingAssetBalance > 1) { - sendingAsset.safeTransfer(receiver, sendingAssetBalance); + LibAsset.transferERC20( + sendingAssetId, + receiver, + sendingAssetBalance + ); } } } diff --git a/src/Helpers/WithdrawablePeriphery.sol b/src/Helpers/WithdrawablePeriphery.sol index 60b35cb2e2..3020ee4e88 100644 --- a/src/Helpers/WithdrawablePeriphery.sol +++ b/src/Helpers/WithdrawablePeriphery.sol @@ -7,6 +7,12 @@ import { LibAsset } from "../Libraries/LibAsset.sol"; import { ExternalCallFailed } from "../Errors/GenericErrors.sol"; import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; +// TODO(EXSC-241): route withdrawToken through LibAsset.transferAsset and add a +// ZeroAmount check. Deferred because this contract is inherited +// by many periphery contracts; bumping it here would force +// redeploys of all inheritors or drift the repo from deployed +// bytecode. Re-enable once EXSC-330 (commit hash stored in +// deploy log) makes that drift recoverable via re-verification. abstract contract WithdrawablePeriphery is TransferrableOwnership { using SafeTransferLib for address; diff --git a/src/Periphery/ReceiverAcrossV3.sol b/src/Periphery/ReceiverAcrossV3.sol index 0250b83fc8..5850fc92ed 100644 --- a/src/Periphery/ReceiverAcrossV3.sol +++ b/src/Periphery/ReceiverAcrossV3.sol @@ -102,6 +102,14 @@ contract ReceiverAcrossV3 is ILiFi, WithdrawablePeriphery { ) {} catch { // send the bridged (and unswapped) funds to receiver address + // TODO(EXSC-241): route through LibAsset.transferERC20 next time + // this file is updated. Reasons: (1) Tron USDT support (fix lives + // in contracts-tron fork via LibAsset), and (2) consistency with + // the rest of the repo. Not done now because ReceiverAcrossV3 is + // not actively developed and not deployed on Tron; even when it + // is, the migration is deferred until EXSC-330 (commit hash + // stored in deploy log) makes the resulting bytecode drift + // recoverable via re-verification. assetId.safeTransfer(receiver, amount); emit LiFiTransferRecovered( diff --git a/src/Periphery/ReceiverChainflip.sol b/src/Periphery/ReceiverChainflip.sol index fa47fca9a1..eae827eba1 100644 --- a/src/Periphery/ReceiverChainflip.sol +++ b/src/Periphery/ReceiverChainflip.sol @@ -131,6 +131,14 @@ contract ReceiverChainflip is ILiFi, WithdrawablePeriphery { { return; } catch { + // TODO(EXSC-241): route through LibAsset.transferERC20 next + // time this file is updated. Reasons: (1) Tron USDT support + // (fix lives in contracts-tron fork via LibAsset), and (2) + // consistency with the rest of the repo. Not done now because + // Chainflip does not support Tron; even when it does, the + // migration is deferred until EXSC-330 (commit hash stored in + // deploy log) makes the resulting bytecode drift recoverable + // via re-verification. actualAssetId.safeTransfer(receiver, amount); emit LiFiTransferRecovered( _transactionId, diff --git a/src/Periphery/ReceiverStargateV2.sol b/src/Periphery/ReceiverStargateV2.sol index 6627387d99..d9b8321569 100644 --- a/src/Periphery/ReceiverStargateV2.sol +++ b/src/Periphery/ReceiverStargateV2.sol @@ -191,6 +191,14 @@ contract ReceiverStargateV2 is if (cacheGasLeft < recoverGas) { // case 2a: not enough gas left to execute calls + // TODO(EXSC-241): route through LibAsset.transferERC20 next + // time this file is updated. Reasons: (1) Tron USDT support + // (fix lives in contracts-tron fork via LibAsset), and (2) + // consistency with the rest of the repo. Not done now because + // Stargate V2 is not on Tron; even when it is, the migration + // is deferred until EXSC-330 (commit hash stored in deploy + // log) makes the resulting bytecode drift recoverable via + // re-verification. token.safeTransfer(receiver, amount); emit LiFiTransferRecovered( @@ -210,6 +218,14 @@ contract ReceiverStargateV2 is gas: cacheGasLeft - recoverGas }(_transactionId, _swapData, assetId, receiver) {} catch { + // TODO(EXSC-241): route through LibAsset.transferERC20 next + // time this file is updated. Reasons: (1) Tron USDT support + // (fix lives in contracts-tron fork via LibAsset), and (2) + // consistency with the rest of the repo. Not done now because + // Stargate V2 is not on Tron; even when it is, the migration + // is deferred until EXSC-330 (commit hash stored in deploy + // log) makes the resulting bytecode drift recoverable via + // re-verification. token.safeTransfer(receiver, amount); emit LiFiTransferRecovered( _transactionId, diff --git a/src/Periphery/TokenWrapper.sol b/src/Periphery/TokenWrapper.sol index cf27399b79..db7dc4ca50 100644 --- a/src/Periphery/TokenWrapper.sol +++ b/src/Periphery/TokenWrapper.sol @@ -11,6 +11,14 @@ import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; import { WETH } from "solady/tokens/WETH.sol"; import { InvalidContract, InvalidConfig } from "../Errors/GenericErrors.sol"; +// TODO(EXSC-241): next time this file is updated, route ERC20 transfers +// through LibAsset instead of SafeTransferLib for codebase +// consistency (the rest of the periphery is moving that +// way). Deferred now because bumping this contract drifts +// the repo from deployed bytecode on chains where it's +// already live. Re-enable once EXSC-330 (commit hash +// stored in deploy log) makes that drift recoverable via +// re-verification. /// @title TokenWrapper /// @author LI.FI (https://li.fi) /// @notice Provides functionality for wrapping and unwrapping tokens