code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L353-L361 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolActions.sol#L73-L87 https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L201-L212

Vulnerability details

Proof of Concept

Function to decrease and increase liquidity are passing amount0Min and amount1Min as zero. This will result in MEV bots sandwiching transactions to extract value from it. In the worst case it will actually return zero or a very small value in wei, and the funds will be lost.

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L353-L361

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolActions.sol#L73-L87

Note that TalosBaseStrategy.deposit() will check the returned amount0 and amount1 are not zero. However, bots may return a small value like 1 wei, which would make the check pass and the value is still virtually zero.

(liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity(
    INonfungiblePositionManager.IncreaseLiquidityParams({
        tokenId: _tokenId,
        amount0Desired: amount0Desired,
        amount1Desired: amount1Desired,
        amount0Min: 0,
        amount1Min: 0,
        deadline: block.timestamp
    })
);

if (amount0 == 0 && amount1 == 0) revert AmountsAreZero();

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L201-L212

Impact

The likelihood of this issue taking place is high since MEV boots are widely deployed on EVM chains. The impact is also high since it will most likely result in losing funds/assets.

Tools Used

Manual review.

Recommended Mitigation Steps

Consider calculating a min amount to be used on amount0Min and amount1Min. Alternatively, consider adding input params so that the caller can choose amount0Min and amount1Min.