Lines of code
<https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Base.sol#L55>
Lack of input validation for ClosePositionParams.amountSwap results in theft of fund
ParticlePositionManager.sol hold two part of fund
but because there is lack of input validation for parameter ClosePositionParams.amountSwap when close the position,
the premium and protocol fee that is stored in the PositionManager can be stolen directly
the struct of ClosePositionParams is below
struct ClosePositionParams {
uint96 lienId;
uint256 amountSwap;
bytes data;
}
the parameter amountSwap is user controlled when liquidation or close the position
function closePosition(DataStruct.ClosePositionParams calldata params) external override nonReentrant {
then in the function _closePosition
function _closePosition(
DataStruct.ClosePositionParams calldata params,
DataCache.ClosePositionCache memory cache,
Lien.Info memory lien,
address borrower
) internal {
// optimistically use the input numbers to swap for repay
/// @dev amountSwap overspend will be caught by refundWithCheck step in below
(cache.amountSpent, cache.amountReceived) = Base.swap(
cache.tokenFrom,
cache.tokenTo,
params.amountSwap,
0, /// @dev we check cache.amountReceived is sufficient to repay LP in below
DEX_AGGREGATOR,
params.data
);
// based on borrowed liquidity, compute the required return amount
/// @dev the from-to swapping direction is reverted compared to openPosition
(cache.amountToAdd, cache.amountFromAdd) = Base.getRequiredRepay(lien.liquidity, lien.tokenId);
if (!lien.zeroForOne) (cache.amountToAdd, cache.amountFromAdd) = (cache.amountFromAdd, cache.amountToAdd);
// the liquidity to add must be no less than the available amount
/// @dev the max available amount contains the tokensOwed, will have another check in below at refundWithCheck
if (
cache.amountFromAdd > cache.collateralFrom + cache.tokenFromPremium - cache.amountSpent ||
cache.amountToAdd > cache.amountReceived + cache.tokenToPremium
) {
revert Errors.InsufficientRepay();
}
the token is swapped to repay LP (lender)
the code only ensure the swapped out amount can repay the LP
but does not the validate parameter amountSwap when swapping
function swap(
address tokenFrom,
address tokenTo,
uint256 amountFrom,
uint256 amountToMinimum,
address dexAggregator,
bytes calldata data
) internal returns (uint256 amountSpent, uint256 amountReceived) {
uint256 balanceFromBefore = IERC20(tokenFrom).balanceOf(address(this));
uint256 balanceToBefore = IERC20(tokenTo).balanceOf(address(this));
if (amountFrom > 0) {
///@dev only allow amountFrom of tokenFrom to be spent by the DEX aggregator
TransferHelper.safeApprove(tokenFrom, dexAggregator, amountFrom);
// solhint-disable-next-line avoid-low-level-calls
(bool success, ) = dexAggregator.call(data);
if (!success) revert Errors.SwapFailed();
TransferHelper.safeApprove(tokenFrom, dexAggregator, 0);
}
amountSpent = balanceFromBefore - IERC20(tokenFrom).balanceOf(address(this));
amountReceived = IERC20(tokenTo).balanceOf(address(this)) - balanceToBefore;
if (amountReceived < amountToMinimum) revert Errors.InsufficientSwap();
}
as we can see
suppose the user wants to close the position and needs to repay LP 100 USDC
the contract hold 1 WETH (otherβs user premium + protocol fee)
the user can use 1inch to swap out WETH to 2200 USDC and then transfer the 100 USDC to the position manager and steal the rest 2100 USDC
for example, if the 1inch v4 is used to pull an arbitrary amount of funds from the caller and execute arbitrary call
<https://polygonscan.com/address/0x1111111254fb6c44bAC0beD2854e76F90643097d#codeL2309-2321>
even if 1inch v5 is used, attacker can still sweep fund out and only ensure the minimum out of fund is repaid and steal the rest fund after aggreagtor executes multi-hop swap
Manual Review
make sure validate the ClosePositionParams.amountSwap, and the code should ensure the user transfer the ClosePositionParams.amountSwap fund into the contract before close the position
Invalid Validation
The text was updated successfully, but these errors were encountered:
All reactions