code-423n4 / 2023-12-particle-findings

2 stars 1 forks source link

lack of slippage protection for `increaseLiquidity`, and `decreaseLiquidity` #41

Closed c4-bot-6 closed 11 months ago

c4-bot-6 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L190-L199 https://github.com/code-423n4/2023-12-particle/blob/main/contracts/libraries/LiquidityPosition.sol#L255-L261

Vulnerability details

Impact

Lack of slippage protection for increasing and decreasing liquidity can cause the liquidity provider to provide liquidity at an unfavorable price. Or the borrower to borrow/repay in a manipulated pool.

Proof of Concept

When adding liquidity eventually calls comes down to LiquidityPosition::increaseLiquidity and decreaseLiquidity which interact with the Uniswap position manager:

LiquidityPosition::increaseLiquidity and decreaseLiquidity:

File: contracts/libraries/LiquidityPosition.sol

178:    function increaseLiquidity(
            // ... params
184:    ) internal returns (uint128 liquidity, uint256 amount0Added, uint256 amount1Added) {
            // ... set approvals

189:        // increase liquidity via position manager
190:        (liquidity, amount0Added, amount1Added) = Base.UNI_POSITION_MANAGER.increaseLiquidity(
191:            INonfungiblePositionManager.IncreaseLiquidityParams({
192:                tokenId: tokenId,
193:                amount0Desired: amount0,
194:                amount1Desired: amount1,
                    // @audit no slippage for which price to add liquidity to
195:                amount0Min: 0,
196:                amount1Min: 0,
197:                deadline: block.timestamp
198:            })
199:        );

            // ... reset approvals
204:    }

...

253:    function decreaseLiquidity(uint256 tokenId, uint128 liquidity) internal returns (uint256 amount0, uint256 amount1) {
254:        (amount0, amount1) = Base.UNI_POSITION_MANAGER.decreaseLiquidity(
255:            INonfungiblePositionManager.DecreaseLiquidityParams({
256:                tokenId: tokenId,
257:                liquidity: liquidity,
                    // @audit no slippage or how much tokens to get when decreasing liquidity
258:                amount0Min: 0,
259:                amount1Min: 0,
260:                deadline: block.timestamp
261:            })
262:        );
263:    }

These are called directly by the liquidity provider through: ParticlePositionManager::increaseLiquidity and ParticlePositionManager::decreaseLiquidity.

As well as indirectly when opening, closing or liquidating a position.

Tools Used

Manual audit

Recommended Mitigation Steps

Consider adding amount0/1Min parameters to ParticlePositionManager::increaseLiquidity and decreaseLiquidity and also through the calls openPosition, closePosition and liquidatePosition.

This would also cover the usage of slot0 in Base::getRequiredRepay as it would enforce a certain amount of token0/1 to be returned when repaying the liquidity.

Assessed type

MEV

c4-judge commented 11 months ago

0xleastwood marked the issue as duplicate of #2

c4-judge commented 11 months ago

0xleastwood marked the issue as satisfactory