code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

Not using slippage parameter in swap() while swapping causes loss of funds #912

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/talos/libraries/PoolActions.sol#L46-L52

Vulnerability details

Impact

While making a swap on UniswapV3 the caller should use the slippage parameter amountOutMinimum parameter to avoid losing funds.

In swapToEqualAmounts() does not use the slippage parameter amountOutMinimum.

File: /src/talos/libraries/PoolActions.sol#L46-L52

    function swapToEqualAmounts(ActionParams memory actionParams, int24 baseThreshold) internal {
        (bool zeroForOne, int256 amountSpecified, uint160 sqrtPriceLimitX96) = actionParams
            .pool
            .getSwapToEqualAmountsParams(
            actionParams.optimizer, actionParams.tickSpacing, baseThreshold, actionParams.token0, actionParams.token1
        );

        //Swap imbalanced token as long as we haven't used the entire amountSpecified and haven't reached the price limit
        actionParams.pool.swap(
            address(this),
            zeroForOne,
            amountSpecified,
            sqrtPriceLimitX96,
            abi.encode(SwapCallbackData({zeroForOne: zeroForOne}))
        );
    }

amountOutMinimum is used to specify the minimum amount of tokens the caller wants to be returned from a swap. Using amountOutMinimum tells the swap that the caller will accept a minimum amount of output tokens from the swap, opening up the user to a catastrophic loss of funds via MEV bot sandwich attacks.

Proof of Concept

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/talos/libraries/PoolActions.sol#L46-L52

Tools Used

Manual review

Recommended Mitigation Steps

Use/Introduce parameters amountOutMinimum to avoid loss of funds.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #177

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-judge commented 1 year ago

trust1995 changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

trust1995 changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

trust1995 changed the severity to 2 (Med Risk)