Lines of code
<https://github.com/code-423n4/2023-12-particle/blob/a3af40839b24aa13f5764d4f84933dbfa8bc8134/contracts/libraries/Base.sol#L55&gt;

Vulnerability details

Impact

Lack of input validation for ClosePositionParams.amountSwap results in theft of fund

Proof of Concept

ParticlePositionManager.sol hold two part of fund

1. the contract hold premium added by borrower
2. the contract hold protocol fee before protocol withdraw it

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&gt;

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

Tools Used

Manual Review

Recommended Mitigation Steps

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

Assessed type

Invalid Validation

---

The text was updated successfully, but these errors were encountered:

All reactions