Incorrect approve call

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L200

Vulnerability details

Impact

Hanging approval, for tokenAAmount - tokenAAmountMin amount, next addLiquidity call will fail for tokens like USDT, because it will try to set new approval, without resetting it to 0 first

Proof of Concept

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

Here, it is not guaranteed that tokenAAmount of amount will be used up in add liquidity call since tokenAAmountMin is minimum which is guaranteed to be used. Hence, this will lead to hanging approval for tokenAAmount - tokenAAmountMin, and when next call happen for addLiquidity again, for tokens like USDT it will fail, because current UDST approval is non zero, and when again a non zero approve call happen, it will fail.

Tools Used

VS Code

Recommended Mitigation Steps

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

Assessed type

Error

[c4-pre-sort]

bytes032 marked the issue as duplicate of #928

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as not a duplicate

[c4-pre-sort]

bytes032 marked the issue as duplicate of #1455

[c4-pre-sort]

bytes032 marked the issue as duplicate of #1782

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory:
Out of scope