UniV2LiquidityAmo - Incorrect use of safeApprove

[code423n4]

Lines of code

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

Vulnerability details

Impact

safeApprove will revert when there's residual approved amount from past liquidity addition, thus addLiquidity will not work.

Proof of Concept

SafeERC20's safeApprove function checkes if there's already approved amount and if it does, it does revert.

function safeApprove(IERC20 token, address spender, uint256 value) internal {
    // safeApprove should only be called when setting an initial allowance,
    // or when resetting it to zero. To increase and decrease it, use
    // 'safeIncreaseAllowance' and 'safeDecreaseAllowance'
    require(
        (value == 0) || (token.allowance(address(this), spender) == 0),
        "SafeERC20: approve from non-zero to non-zero allowance"
    );
    _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));
}

Tools Used

Manual Review

Recommended Mitigation Steps

Reset approved amount before using safeApprove.

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

ERC20

[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 #1455

[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