function swapTokensGeneric(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) public payable {
uint256 receivingAssetIdBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId);
// Swap
_executeSwaps(_lifiData, _swapData);
uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - receivingAssetIdBalance;
LibAsset.transferAsset(_lifiData.receivingAssetId, payable(msg.sender), postSwapBalance);
function swap(bytes32 transactionId, SwapData calldata _swapData) internal {
uint256 fromAmount = _swapData.fromAmount;
uint256 toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId);
address fromAssetId = _swapData.sendingAssetId;
if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
}
if (!LibAsset.isNativeAsset(fromAssetId)) {
LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
}
// solhint-disable-next-line avoid-low-level-calls
(bool success, bytes memory res) = _swapData.callTo.call{ value: msg.value }(_swapData.callData);
if (!success) {
string memory reason = LibUtil.getRevertMsg(res);
revert(reason);
}
toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId) - toAmount;
emit AssetSwapped(
transactionId,
_swapData.callTo,
_swapData.sendingAssetId,
_swapData.receivingAssetId,
fromAmount,
toAmount,
block.timestamp
);
}
The Swapper allow arbitrary _swapData (0x style), this makes it possible for a attacker to steal the funds in the contract.
Based on the context, we beleive itβs possible that the contract can hold funds.
The funds can be the refunds of failed orders, or fee rebates from bridging or dex aggregators, etc.
See also the permissioned WithdrawFacet.
Given:
The attacker can submit a swapTokensGeneric() with USDT as receivingAssetId with the following SwapData[]:
As a result, the attacker will receive ~100 USDT with 0 USDC paid.
Given that Swapper is a standlone module that can be and should be deployed as a standalone contract, we suggest spin it off from the diamond so that it can no longer access the funds in the diamond contract.
The text was updated successfully, but these errors were encountered:
All reactions