code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

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

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

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 commented 1 year ago

141345 marked the issue as duplicate of #78

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #260

c4-judge commented 1 year ago

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

c4-judge commented 1 year ago

gzeon-c4 marked the issue as grade-c