addLiquidity does not reset approval if not all tokens were added to liquidity pool

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L189-L250
https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L155-L211

Vulnerability details

Impact

Unused approval amounts can accrue over time, leading to large Uniswap approval.

Proof of Concept

In addLiquidity(), UniV2LiquidityAmo contract approve UniswapV2 AMM router to spend amont of tokenA and tokenB:

IERC20WithBurn(addresses.tokenA).safeApprove(
    addresses.ammRouter,
    tokenAAmount
);
IERC20WithBurn(addresses.tokenB).safeApprove(
    addresses.ammRouter,
    tokenBAmount
);

Then call AMM router to add liqiudity:

(tokenAUsed, tokenBUsed, lpReceived) = IUniswapV2Router(addresses.ammRouter)
      .addLiquidity(
        addresses.tokenA,
        addresses.tokenB,
        tokenAAmount,
        tokenBAmount,
        tokenAAmountMin,
        tokenBAmountMin,
        address(this),
        block.timestamp + 1
      );

But when tokenAUsed < tokenAAmount or tokenBUsed < tokenBAmount, unused approval still exists and contract doesn't reset to zero.
For UniV3LiquidityAmo.sol is same way as UniV2LiquidityAmo.sol

Recommended Mitigation Steps

If tokenAUsed < tokenAAmount or tokenBUsed < tokenBAmount reset AMM router approval to zero.

Similar Issues:

Malt Finance contest (M-23)

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as duplicate of #928

[c4-pre-sort]

bytes032 marked the issue as not a duplicate

[c4-pre-sort]

bytes032 marked the issue as duplicate of #1782

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory:
Out of scope