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

24 stars 13 forks source link

Liquidity providers may lose funds when initialising a strategy #776

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/base/TalosBaseStrategy.sol#L135-L149 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/talos/base/TalosBaseStrategy.sol#L102

Vulnerability details

Summary

Liquidity providers may lose funds when initialising a strategy

Vulnerability Detail

Liquidity providers may lose a portion of provided liquidity in either of the pair tokens when creating a new position. The init function on TalosBaseStrategy.sol does not check for price volatility before providing / swapping liquidity. Unlike in the rest of interactions with Uniswap where the modifier checkDeviation checks for price volatility before providing / swapping liquidity. While amount0Desired & amount1Desired specify the amount of tokens the user wants to initialise the position with. amount0Min & amount1Min should not be set to 0 in order to protect from slippage when adding liquidity.

/// TalosBaseStrategy.sol
function init(uint256 amount0Desired, uint256 amount1Desired, address receiver)
        external
        virtual
        nonReentrant
        returns (uint256 shares, uint256 amount0, uint256 amount1)
    {
        ...
    (_tokenId, _liquidity, amount0, amount1) = _nonfungiblePositionManager.mint(
        INonfungiblePositionManager.MintParams({
           token0: address(_token0),
           token1: address(_token1),
           fee: poolFee,
           tickLower: tickLower,
           tickUpper: tickUpper,
           amount0Desired: amount0Desired,
           amount1Desired: amount1Desired,
           amount0Min: 0, /// @audit this could lead to loss of value
           amount1Min: 0,
           recipient: address(this),
           deadline: block.timestamp
        })
     );
   ...
 );

Impact

High, a strategy may be initialised with wrong liquidity amounts, initialiser may loss funds due to providing liquidity in the wrong ratio.

Tools Used

Manual review, foundry

Recommended Mitigation Steps

Add the modifier checkDeviation to the init function on TalosBaseStrategy.sol to prevent price manipulation, or use the pool twap oracle to get accurate minimum amounts for the swap.

Assessed type

Uniswap

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #271

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory