Skip to content

Commit

Permalink
(fix): adds calldata extraction handling for GenericSwapV3 single swa…
Browse files Browse the repository at this point in the history 
…p calls (CallDataVerificationFacet v1.2.0) [CalldataVerificationFacet v1.2.0,IAcrossSpokePool v1.0.0] (#767)

* adds extraction handling for GenericSwapV3 single calldata

* push test coverage to 100%

* remove lcov-filtered.info

* minor update

* remove console import

* gas optimization (audit issue #7)

* adds handling for StargateV2 destination calls (audit issue #8)

* gas optimization in extractGenericSwapParameters (audit issue #6)

* update min callData length check (audit issue #4)

* gas optimization in extractNonEVMAddress (audit issue #3)

* update comments, fix typo (audit issue#2)

* adds handling for AcrossV3 destination calls (audit issue #9)

* typo fixed in comment

* gas optimization

* updates audit log and adds audit report

* update log

---------

Co-authored-by: Ed Zynda <[email protected]>
Co-authored-by: Ed Zynda <[email protected]>
  • Loading branch information
@0xDEnYO @ezynda3
3 people authored Jan 11, 2025
1 parent f83f108 commit ed3e38f
Show file tree
Hide file tree
Showing 5 changed files with 463 additions and 44 deletions.
17 changes: 17 additions & 0 deletions audit/auditLog.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
"auditReportPath": "./audit/reports/2024.08.14_StargateFacetV2_ReAudit.pdf",
"auditCommitHash": "d622002440317580b5d0fb90ef22b839d84957e2"
},
"audit20240902": {
"auditCompletedOn": "02.09.2024",
"auditedBy": "Sujith Somraaj (individual security researcher)",
"auditorGitHandle": "sujithsomraaj",
"auditReportPath": "./audit/reports/2024.09.02_CalldataVerificationFacet.pdf",
"auditCommitHash": "374d066a2d4ffd98eb6042cac7a2dd4aa0bf2ff8"
},
"audit20240913": {
"auditCompletedOn": "13.09.2024",
"auditedBy": "Sujith Somraaj (individual security researcher)",
Expand Down Expand Up @@ -137,6 +144,11 @@
"audit20241206"
]
},
"CalldataVerificationFacet": {
"1.2.0": [
"audit20240902"
]
},
"DeBridgeDlnFacet": {
"1.0.0": [
"audit20241205"
Expand Down Expand Up @@ -181,6 +193,11 @@
"audit20250110_1"
]
},
"IAcrossSpokePool": {
"1.0.0": [
"audit20250106"
]
},
"IGasZip": {
"1.0.0": [
"audit20241107"
Expand Down
Binary file added audit/reports/2024.09.02_CalldataVerificationFacet.pdf
View file
Binary file not shown.
209 changes: 170 additions & 39 deletions src/Facets/CalldataVerificationFacet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@ pragma solidity ^0.8.17;
import { ILiFi } from "../Interfaces/ILiFi.sol";
import { LibSwap } from "../Libraries/LibSwap.sol";
import { AmarokFacet } from "./AmarokFacet.sol";
import { AcrossFacetV3 } from "./AcrossFacetV3.sol";
import { StargateFacetV2 } from "./StargateFacetV2.sol";
import { StargateFacet } from "./StargateFacet.sol";
import { AcrossFacetV3 } from "./AcrossFacetV3.sol";
import { CelerIMFacetBase, CelerIM } from "lifi/Helpers/CelerIMFacetBase.sol";
import { StandardizedCallFacet } from "lifi/Facets/StandardizedCallFacet.sol";
import { LibBytes } from "../Libraries/LibBytes.sol";
import { GenericSwapFacetV3 } from "lifi/Facets/GenericSwapFacetV3.sol";
import { InvalidCallData } from "../Errors/GenericErrors.sol";

/// @title CalldataVerificationFacet
/// @author LI.FI (https://li.fi)
/// @notice Provides functionality for verifying calldata
/// @custom:version 1.1.2
/// @custom:version 1.2.0
contract CalldataVerificationFacet {
using LibBytes for bytes;

Expand Down Expand Up @@ -67,7 +71,7 @@ contract CalldataVerificationFacet {
function extractMainParameters(
bytes calldata data
)
public
external
pure
returns (
string memory bridge,
Expand Down Expand Up @@ -101,24 +105,25 @@ contract CalldataVerificationFacet {
);
}

// @notice Extracts the non-EVM address from the calldata
// @param data The calldata to extract the non-EVM address from
// @return nonEVMAddress The non-EVM address extracted from the calldata
/// @notice Extracts the non-EVM address from the calldata
/// @param data The calldata to extract the non-EVM address from
/// @return nonEVMAddress The non-EVM address extracted from the calldata
function extractNonEVMAddress(
bytes calldata data
) external pure returns (bytes32 nonEVMAddress) {
bytes memory callData = data;
ILiFi.BridgeData memory bridgeData = _extractBridgeData(data);
bytes memory callData;

if (
bytes4(data[:4]) == StandardizedCallFacet.standardizedCall.selector
) {
// standardizedCall
callData = abi.decode(data[4:], (bytes));
} else {
callData = data;
}

// Non-EVM address is always the first parameter of bridge specific data
if (bridgeData.hasSourceSwaps) {
if (_extractBridgeData(data).hasSourceSwaps) {
assembly {
let offset := mload(add(callData, 0x64)) // Get the offset of the bridge specific data
nonEVMAddress := mload(add(callData, add(offset, 0x24))) // Get the non-EVM address
Expand Down Expand Up @@ -151,30 +156,63 @@ contract CalldataVerificationFacet {
uint256 receivingAmount
)
{
// valid callData for a genericSwap call should have at least 484 bytes:
// Function selector: 4 bytes
// _transactionId: 32 bytes
// _integrator: 64 bytes
// _referrer: 64 bytes
// _receiver: 32 bytes
// _minAmountOut: 32 bytes
// _swapData: 256 bytes
if (data.length <= 484) {
revert InvalidCallData();
}

LibSwap.SwapData[] memory swapData;
bytes memory callData = data;
bytes memory callData;
bytes4 functionSelector = bytes4(data[:4]);

// check if this is a call via StandardizedCallFacet
if (
bytes4(data[:4]) == StandardizedCallFacet.standardizedCall.selector
functionSelector == StandardizedCallFacet.standardizedCall.selector
) {
// standardizedCall
callData = abi.decode(data[4:], (bytes));
// extract nested function selector and calldata
// will always start at position 68
functionSelector = bytes4(data[68:72]);
callData = data[68:];
// callData = abi.decode(data[4:], (bytes)); // this one is also valid, even though the calldata differs slightly (add. padding)
} else {
callData = data;
}

if (
functionSelector ==
GenericSwapFacetV3.swapTokensSingleV3ERC20ToERC20.selector ||
functionSelector ==
GenericSwapFacetV3.swapTokensSingleV3ERC20ToNative.selector ||
functionSelector ==
GenericSwapFacetV3.swapTokensSingleV3NativeToERC20.selector
) {
// single swap
swapData = new LibSwap.SwapData[](1);

// extract parameters from calldata
(, , , receiver, receivingAmount, swapData[0]) = abi.decode(
callData.slice(4, callData.length - 4),
(bytes32, string, string, address, uint256, LibSwap.SwapData)
);
} else {
// multi swap or GenericSwap V1 call
(, , , receiver, receivingAmount, swapData) = abi.decode(
callData.slice(4, callData.length - 4),
(bytes32, string, string, address, uint256, LibSwap.SwapData[])
);
}
(, , , receiver, receivingAmount, swapData) = abi.decode(
callData.slice(4, callData.length - 4),
(bytes32, string, string, address, uint256, LibSwap.SwapData[])
);

// extract missing return parameters from swapData
sendingAssetId = swapData[0].sendingAssetId;
amount = swapData[0].fromAmount;
receivingAssetId = swapData[swapData.length - 1].receivingAssetId;
return (
sendingAssetId,
amount,
receiver,
receivingAssetId,
receivingAmount
);
}

/// @notice Validates the calldata
Expand All @@ -189,7 +227,7 @@ contract CalldataVerificationFacet {
/// or type(uint256).max to ignore
/// @param hasSourceSwaps Whether the calldata has source swaps
/// @param hasDestinationCall Whether the calldata has a destination call
/// @return isValid Whether the calldata is validate
/// @return isValid Returns true if the calldata is valid
function validateCalldata(
bytes calldata data,
string calldata bridge,
Expand All @@ -200,22 +238,14 @@ contract CalldataVerificationFacet {
bool hasSourceSwaps,
bool hasDestinationCall
) external pure returns (bool isValid) {
ILiFi.BridgeData memory bridgeData;
(
bridgeData.bridge,
bridgeData.sendingAssetId,
bridgeData.receiver,
bridgeData.minAmount,
bridgeData.destinationChainId,
bridgeData.hasSourceSwaps,
bridgeData.hasDestinationCall
) = extractMainParameters(data);
ILiFi.BridgeData memory bridgeData = _extractBridgeData(data);

bytes32 bridgeNameHash = keccak256(abi.encodePacked(bridge));
return
// Check bridge
(keccak256(abi.encodePacked(bridge)) ==
keccak256(abi.encodePacked("")) ||
(bridgeNameHash == keccak256(abi.encodePacked("")) ||
keccak256(abi.encodePacked(bridgeData.bridge)) ==
keccak256(abi.encodePacked(bridge))) &&
bridgeNameHash) &&
// Check sendingAssetId
(sendingAssetId == 0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF ||
bridgeData.sendingAssetId == sendingAssetId) &&
Expand All @@ -235,9 +265,9 @@ contract CalldataVerificationFacet {

/// @notice Validates the destination calldata
/// @param data The calldata to validate
/// @param callTo The call to address to validate
/// @param callTo The callTo address to validate
/// @param dstCalldata The destination calldata to validate
/// @return isValid Returns true if the calldata matches with the provided parameters
/// @return isValid Returns true if the calldata is valid
function validateDestinationCalldata(
bytes calldata data,
bytes calldata callTo,
Expand All @@ -254,6 +284,7 @@ contract CalldataVerificationFacet {

bytes4 selector = abi.decode(callData, (bytes4));

// ---------------------------------------
// Case: Amarok
if (selector == AmarokFacet.startBridgeTokensViaAmarok.selector) {
(, AmarokFacet.AmarokData memory amarokData) = abi.decode(
Expand All @@ -277,6 +308,7 @@ contract CalldataVerificationFacet {
abi.decode(callTo, (address)) == amarokData.callTo;
}

// ---------------------------------------
// Case: Stargate
if (selector == StargateFacet.startBridgeTokensViaStargate.selector) {
(, StargateFacet.StargateData memory stargateData) = abi.decode(
Expand All @@ -303,6 +335,51 @@ contract CalldataVerificationFacet {
keccak256(dstCalldata) == keccak256(stargateData.callData) &&
keccak256(callTo) == keccak256(stargateData.callTo);
}

// ---------------------------------------
// Case: StargateV2

if (
selector == StargateFacetV2.startBridgeTokensViaStargate.selector
) {
(, StargateFacetV2.StargateData memory stargateDataV2) = abi
.decode(
callData.slice(4, callData.length - 4),
(ILiFi.BridgeData, StargateFacetV2.StargateData)
);

return
keccak256(dstCalldata) ==
keccak256(stargateDataV2.sendParams.composeMsg) &&
_compareBytesToBytes32CallTo(
callTo,
stargateDataV2.sendParams.to
);
}
if (
selector ==
StargateFacetV2.swapAndStartBridgeTokensViaStargate.selector
) {
(, , StargateFacetV2.StargateData memory stargateDataV2) = abi
.decode(
callData.slice(4, callData.length - 4),
(
ILiFi.BridgeData,
LibSwap.SwapData[],
StargateFacetV2.StargateData
)
);

return
keccak256(dstCalldata) ==
keccak256(stargateDataV2.sendParams.composeMsg) &&
_compareBytesToBytes32CallTo(
callTo,
stargateDataV2.sendParams.to
);
}

// ---------------------------------------
// Case: Celer
if (
selector == CelerIMFacetBase.startBridgeTokensViaCelerIM.selector
Expand Down Expand Up @@ -357,6 +434,37 @@ contract CalldataVerificationFacet {
keccak256(abi.encode(acrossV3Data.receiverAddress));
}

// ---------------------------------------
// Case: AcrossV3
if (selector == AcrossFacetV3.startBridgeTokensViaAcrossV3.selector) {
(, AcrossFacetV3.AcrossV3Data memory acrossV3Data) = abi.decode(
callData.slice(4, callData.length - 4),
(ILiFi.BridgeData, AcrossFacetV3.AcrossV3Data)
);

return
keccak256(dstCalldata) == keccak256(acrossV3Data.message) &&
keccak256(callTo) ==
keccak256(abi.encode(acrossV3Data.receiverAddress));
}
if (
selector ==
AcrossFacetV3.swapAndStartBridgeTokensViaAcrossV3.selector
) {
(, , AcrossFacetV3.AcrossV3Data memory acrossV3Data) = abi.decode(
callData.slice(4, callData.length - 4),
(
ILiFi.BridgeData,
LibSwap.SwapData[],
AcrossFacetV3.AcrossV3Data
)
);
return
keccak256(dstCalldata) == keccak256(acrossV3Data.message) &&
keccak256(callTo) ==
keccak256(abi.encode(acrossV3Data.receiverAddress));
}

// All other cases
return false;
}
Expand Down Expand Up @@ -407,4 +515,27 @@ contract CalldataVerificationFacet {
(ILiFi.BridgeData, LibSwap.SwapData[])
);
}

function _compareBytesToBytes32CallTo(
bytes memory callTo,
bytes32 callToBytes32
) private pure returns (bool) {
require(
callTo.length >= 20,
"Invalid callTo length; expected at least 20 bytes"
);

// Convert bytes to address type from callTo
address callToAddress;
assembly {
callToAddress := mload(add(callTo, 32))
}

// Convert callToBytes32 to address type and compare them
address callToAddressFromBytes32 = address(
uint160(uint256(callToBytes32))
);

return callToAddress == callToAddressFromBytes32;
}
}
2 changes: 1 addition & 1 deletion src/Interfaces/IAcrossSpokePool.sol
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
/// @custom:version 1.0.0
pragma solidity ^0.8.17;

/// @custom:version 1.0.0
interface IAcrossSpokePool {
function deposit(
address recipient, // Recipient address
Expand Down
Loading

0 comments on commit ed3e38f

Please sign in to comment.