no slippage protection in decreaseLiquidity in withdrawOptionAssets and withdrawOptions

[code423n4]

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.
no slippage protection in decreaseLiquidity in withdrawOptionAssets and withdrawOptions

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
According to uniswap docs

In production, amount0Min and amount1Min should be adjusted to create slippage protections.

but there is a passing 0,0 withdraw(amount, 0, 0), so there will be no slippage protections

    function withdrawOptions(
        uint poolId,
        address optionAddress,
        uint amount
    )
    external
    {
        (ILendingPool LP,,, address token0, address token1) = getPoolAddresses(poolId);
        require(LP.getReserveData(optionAddress).aTokenAddress != address(0x0), "OPM: Invalid Address");
        PMWithdraw(LP, msg.sender, optionAddress, amount);

        // Get output amounts from oracle to avoid sandwich
        (uint amount0, uint amount1) = TokenisableRange(optionAddress).withdraw(amount, 0, 0); //@audit passing 0,0 instead of amount0Min, amount1Min
        checkExpectedBalances(optionAddress, amount, amount0, amount1);
        cleanup(LP, msg.sender, optionAddress);
        cleanup(LP, msg.sender, token0);
        cleanup(LP, msg.sender, token1);
    }

PositionManager/OptionsPositionManager.sol#L424

    function withdrawOptionAssets(
        uint poolId,
        address flashAsset,
        uint256 flashAmount,
        address sourceSwap,
        address user
    )
    private returns (bool result)
    {
...
    (uint256 amount0, uint256 amount1) = TokenisableRange(flashAsset).withdraw(flashAmount, 0, 0);//@audit should be slippage protection
...

PositionManager/OptionsPositionManager.sol#L135

    function withdraw(uint256 lp, uint256 amount0Min, uint256 amount1Min) external nonReentrant returns (uint256 removed0, uint256 removed1) {
        claimFee();
        uint removedLiquidity = uint(liquidity) * lp / totalSupply();
        (removed0, removed1) = POS_MGR.decreaseLiquidity( //@audit amount0Min, amount1Min should be non 0 above
            INonfungiblePositionManager.DecreaseLiquidityParams({
                tokenId: tokenId,
                liquidity: uint128(removedLiquidity),
                amount0Min: amount0Min,
                amount1Min: amount1Min,
                deadline: block.timestamp
            })
        );

contracts/TokenisableRange.sol#L295

Tools Used

Recommended Mitigation Steps

Allow users to specific slippage protection according to uniswap docs and the same for buyOptions encode amount0Min, amount1Min to pass to flashloan

    function withdrawOptions(
        uint poolId,
        address optionAddress,
        uint amount,
+        uint amount0Min,
+        uint amount1Min,
    )
    external
    {
        (ILendingPool LP,,, address token0, address token1) = getPoolAddresses(poolId);
        require(LP.getReserveData(optionAddress).aTokenAddress != address(0x0), "OPM: Invalid Address");
        PMWithdraw(LP, msg.sender, optionAddress, amount);

        // Get output amounts from oracle to avoid sandwich
-        (uint amount0, uint amount1) = TokenisableRange(optionAddress).withdraw(amount, 0, 0);
+        (uint amount0, uint amount1) = TokenisableRange(optionAddress).withdraw(amount, amount0Min, amount1Min);
        checkExpectedBalances(optionAddress, amount, amount0, amount1);
        cleanup(LP, msg.sender, optionAddress);
        cleanup(LP, msg.sender, token0);
        cleanup(LP, msg.sender, token1);
    }

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-pre-sort]

141345 marked the issue as duplicate of #260

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Insufficient proof

[c4-judge]

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

[c4-judge]

gzeon-c4 marked the issue as grade-c