sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

kaysoft - Loss of funds due to lack of slippage protection on increaseLiquidity and decreaseLiquidity. #94

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

kaysoft

medium

Loss of funds due to lack of slippage protection on increaseLiquidity and decreaseLiquidity.

Summary

The increaseLiquidity and decreaseLiquidity functions have no slippage protection in place because the amount0Min and amount1Min are set to zero.

Vulnerability Detail

Slippage helps protect funds from MEV sandwich attack during increaseLiquidity and removeliquidity operations and it is ensured with the amountMin values that the user is ready to take.

(burned0, burned1) = UNISWAP_NFT.decreaseLiquidity(
            IUniswapNFT.DecreaseLiquidityParams({
                tokenId: tokenId,
                liquidity: liquidity,
                amount0Min: 0,//@audit no slippage protection.
                amount1Min: 0,
                deadline: block.timestamp
            })
        );

Impact

Loss of funds due to MEV sandwich attack

Code Snippet

Tool used

Manual Review

Recommendation

Allow amount0Min and amount1Min as input parameters to allow users specify the minimum amount they are willing to take and not met, the transaction will revert.

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because BoostManager and UniswapNFTManager are out of scope

tsvetanovv commented:

These contracts is out of scope

MohammedRizwan commented:

valid