Does not revoke approval for some addresses after changing them on ReLPContract contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L150-L163

Vulnerability details

The contract ReLPContract allows an admin user changes a list of addresses of a addresses state via a function setAddresses includes pair, dopexAMMRouter, tokenA and tokenB, ammRouter, and so on.

After setting these addresses to a new list, the contract will approve the maximum allowance of the pair, tokenA, and tokenB addresses for the ammRouter address.

Everything is okay if the admin calls the first time. But if after that, the admin calls to change to a new ammRouter address then the contract does not change the allowance of the old address to zero.

This bug will able to increase risks. If any bad thing occurs such as hack accidents in this old ammRouter address will permit the drain of all funds in the pair, tokenA, and tokenB addresses.

The vulnerable code:

function setAddresses(
    address _tokenA,
    address _tokenB,
    address _pair,
    address _rdpxV2Core,
    address _tokenAReserve,
    address _amo,
    address _rdpxOracle,
    address _ammFactory,
    address _ammRouter
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    require(
      _tokenA != address(0) &&
        _tokenB != address(0) &&
        _pair != address(0) &&
        _rdpxV2Core != address(0) &&
        _tokenAReserve != address(0) &&
        _amo != address(0) &&
        _rdpxOracle != address(0) &&
        _ammFactory != address(0) &&
        _ammRouter != address(0),
      "reLPContract: address cannot be 0"
    );
    addresses = Addresses({
      tokenA: _tokenA,
      tokenB: _tokenB,
      pair: _pair,
      rdpxV2Core: _rdpxV2Core,
      tokenAReserve: _tokenAReserve,
      amo: _amo,
      rdpxOracle: _rdpxOracle,
      ammFactory: _ammFactory,
      ammRouter: _ammRouter
    });

    IERC20WithBurn(addresses.pair).safeApprove(
      addresses.ammRouter,
      type(uint256).max
    );

    IERC20WithBurn(addresses.tokenA).safeApprove(
      addresses.ammRouter,
      type(uint256).max
    );

    IERC20WithBurn(addresses.tokenB).safeApprove(
      addresses.ammRouter,
      type(uint256).max
    );
  }

I recommended adding a line to change the allowance of the old ammRouter address to zero.

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as duplicate of #928

[c4-pre-sort]

bytes032 marked the issue as low quality report

[c4-pre-sort]

bytes032 marked the issue as not a duplicate

[c4-pre-sort]

bytes032 marked the issue as duplicate of #1662

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory:
Out of scope