withdrawOptionAssets in OptionsPositionManager.sol doesn't use any slippage protection on withdrawing liquidity from UniswapV3

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L135

Vulnerability details

Impact

The function withdrawOptionAssets is used in executeBuyOptions, and it decreases liquidity provided on UniswapV3 on each asset created, but the withdraw used is setting amount0Min and amount1Min to 0 which can make the protocol susceptible to sandwich attacks.

Proof of Concept

Because of the fact that the protocol decreases liquidity without any slippage or timestamp protection when the withdrawOptionAssets is called, it can lead to losing some amount of funds every time, which could hurt the protocol and the users.

Tools Used

Manual review

Recommended Mitigation Steps

Consider implementing some way of slippage protection in the withdrawOptionAssets function, even if it is a bigger slippage, since you are calling withdrawOptionAssets in a loop and you want the function to succeed every time.

eps

Assessed type

MEV

[c4-pre-sort]

141345 marked the issue as duplicate of #78

[c4-pre-sort]

141345 marked the issue as duplicate of #260

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)

[c4-judge]

gzeon-c4 marked the issue as grade-c