Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lack of slippage protection for increaseLiquidity, and decreaseLiquidity #41

Closed
c4-bot-6 opened this issue Dec 21, 2023 · 2 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-2 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-bot-6
Copy link
Contributor

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-bot-6 c4-bot-6 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 21, 2023
c4-bot-5 added a commit that referenced this issue Dec 21, 2023
@c4-judge
Copy link
Contributor

0xleastwood marked the issue as duplicate of #2

@c4-judge c4-judge added duplicate-2 satisfactory satisfies C4 submission criteria; eligible for awards labels Dec 21, 2023
@c4-judge
Copy link
Contributor

0xleastwood marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-2 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants