Incorrect setting of flow limit can lead to DoS

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-08-axelar-network/blob/main/interchain-token-service/contracts/utils/FlowLimit.sol#L103-L105

Vulnerability details

Impact

Incorrect setting of flow limit can lead to DoS due to:

panic: arithmetic underflow or overflow

Proof of Concept

when an interchain token is deployed and initialSupply is greater than zero:

if (initialSupply > 0) {
    IInterchainToken token = IInterchainToken(interchainTokenService.interchainTokenAddress(tokenId));
    ITokenManager tokenManager = ITokenManager(interchainTokenService.tokenManagerAddress(tokenId));

    token.mint(sender, initialSupply);

    token.transferMintership(minter);
    tokenManager.removeFlowLimiter(address(this));
    // If minter == address(0), we still set it as a flow limiter for consistency with the remote token manager.
    tokenManager.addFlowLimiter(minter);  <@

    tokenManager.transferOperatorship(minter);
}

Minter address is assign FlowLimiter role otherwise interchainTokenService is assign flowlimit role.

Minter role can set any flowlimit value for a specific tokenManager:

function setFlowLimit(uint256 flowLimit_) external onlyRole(uint8(Roles.FLOW_LIMITER)) {
    // slither-disable-next-line var-read-using-this
    _setFlowLimit(flowLimit_, this.interchainTokenId());
}

However the flowLimit value is not checked , once type(uint256).max is used as the flowlimit value is can result in DOS.
This may go against the original intent of the setter.

test:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console} from "forge-std/Test.sol";
//import proxy
import "contracts/proxies/InterchainProxy.sol";
import "contracts/proxies/TokenManagerProxy.sol";

//import logic
import "contracts/token-manager/TokenManager.sol";
import "contracts/InterchainTokenService.sol";
import "contracts/interchain-token/InterchainToken.sol";

import "contracts/TokenHandler.sol";
import "contracts/InterchainTokenFactory.sol";
import "contracts/utils/InterchainTokenDeployer.sol";
import "contracts/utils/TokenManagerDeployer.sol";
import "contracts/utils/GatewayCaller.sol";

import { MockGateway } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/test/mocks/MockGateway.sol';
import { AxelarGasService } from '@axelar-network/axelar-cgp-solidity/contracts/gas-service/AxelarGasService.sol';
import { Create3Deployer } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/Create3Deployer.sol';
import { CreateDeploy } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/deploy/CreateDeploy.sol';
import { AddressBytes } from '@axelar-network/axelar-gmp-sdk-solidity/contracts/libs/AddressBytes.sol';
contract CounterTest is Test {
    MockGateway gateway;
    AxelarGasService gasService;
    TokenHandler tokenHandler;
    InterchainTokenFactory tokenFactory;
    InterchainToken ict;
    InterchainToken ict2;
    InterchainTokenDeployer ictd;
    InterchainTokenDeployer ictd2;
    TokenManager tm;
    TokenManager tm2;
    TokenManagerDeployer tmd;
    InterchainTokenService icts;
    InterchainTokenService icts2;
    GatewayCaller gc;
    using AddressBytes for bytes;
    using AddressBytes for address;
    function _create3Address(address deployer,bytes32 deploySalt) internal view returns (address deployed) {
        address deployer = address(
            uint160(uint256(keccak256(abi.encodePacked(hex'ff', deployer, deploySalt, keccak256(type(CreateDeploy).creationCode)))))
        );

        deployed = address(uint160(uint256(keccak256(abi.encodePacked(hex'd6_94', deployer, hex'01')))));
    }

    function setUp() public {
        gateway = new MockGateway();
        gasService = new AxelarGasService(address(this));

        //use deployer.
        Create3Deployer deployerContract = new Create3Deployer();

        bytes32 salt = keccak256(abi.encode(address(this), bytes32('interchaintokenservice')));
        address InterchainTokenServicePred = _create3Address(address(deployerContract),salt);
        address InterchainTokenServicePred2 = _create3Address(address(deployerContract),keccak256(abi.encode(address(this), bytes32('interchaintokenservice-chain2'))));

        //interchaintoken.
        ict = new InterchainToken(InterchainTokenServicePred);
        ictd = new InterchainTokenDeployer(address(ict));

        ict2 = new InterchainToken(InterchainTokenServicePred2);
        ictd2 = new InterchainTokenDeployer(address(ict2));

        tokenHandler = new TokenHandler(address(gateway));
        tm = new TokenManager(InterchainTokenServicePred);
        tm2 = new TokenManager(InterchainTokenServicePred2);
        tmd = new TokenManagerDeployer();
        gc = new GatewayCaller(address(gateway),address(gasService));
        // address tokenFactoryPred = 0xD6BbDE9174b1CdAa358d2Cf4D57D1a9F7178FBfF;
        address tokenFactoryPred = _create3Address(address(deployerContract),keccak256(abi.encode(address(this), bytes32('interchaintokenFactory'))));

        bytes memory bytecode = abi.encodePacked(type(InterchainTokenService).creationCode, abi.encode(
            address(tmd),
            address(ictd),
            address(gateway),
            address(gasService),
            address(tokenFactoryPred),
            'chain1',
            address(tm),
            address(tokenHandler),
            address(gc)
        ));

        bytes memory bytecode2 = abi.encodePacked(type(InterchainTokenService).creationCode, abi.encode(
            address(tmd),
            address(ictd2),
            address(gateway),
            address(gasService),
            address(tokenFactoryPred),
            'chain2',
            address(tm2),
            address(tokenHandler),
            address(gc)
        ));
        deployerContract.deploy(bytecode,bytes32('interchaintokenservice'));
        icts = InterchainTokenService(InterchainTokenServicePred);

        deployerContract.deploy(bytecode2,bytes32('interchaintokenservice-chain2'));
        icts2 = InterchainTokenService(InterchainTokenServicePred2);

        // tokenFactory = new InterchainTokenFactory(InterchainTokenServicePred);
        bytes memory bytecode3 = abi.encodePacked(type(InterchainTokenFactory).creationCode, abi.encode(
            address(InterchainTokenServicePred)
        ));
        deployerContract.deploy(bytecode3,bytes32('interchaintokenFactory'));
        tokenFactory = InterchainTokenFactory(tokenFactoryPred);
    }

    function testFlowlimitOverflow() public {
        address minter = makeAddr('minter');
        bytes32 tokenId = tokenFactory.deployInterchainToken(bytes32('unique-token-id'),'name','symbol',18,100e18,minter);

        address tokenManager = icts.tokenManagerAddress(tokenId);
        address tokenAddress = icts.validTokenAddress(tokenId);

        vm.prank(minter);
        ITokenManager(tokenManager).setFlowLimit(type(uint256).max);

        uint256 amount = 1;
        //transfer.
        vm.startPrank(address(icts));
        ITokenManager(tokenManager).addFlowOut(amount);
        ITokenManager(tokenManager).addFlowIn(amount);
    }

out:

    ├─ [2552] TokenManagerProxy::addFlowIn(1)
    │   ├─ [395] InterchainTokenService::tokenManagerImplementation(0) [staticcall]
    │   │   └─ ← [Return] TokenManager: [0x03A6a84cD762D9707A21605b548aaaB891562aAb]
    │   ├─ [1319] TokenManager::addFlowIn(1) [delegatecall]
    │   │   └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
    │   └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
    └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)

Tools Used

Foundry

Recommended Mitigation Steps

limit the value of flowLimit

Assessed type

DoS

[milapsheth]

The report is valid. Although we consider the severity to be QA. A flow limit of 0 is meant to be set to allow non rate limited transfers (which is the default) and not uint256.max. An operator incorrectly setting the flow limit for their own token can easily overwrite it again if needed (and it only the token operated by them). ITS operator can also overwrite the flow limit if needed. Still, we'll consider making the logic robust against overflows

[alex-ppg]

The Warden outlines how unrestricted configuration of the flow limit of a token can cause flow limit checks to fail due to an addition overflow if it is configured to a significantly high value.

I am inclined to agree with the Sponsor as a flow limit of 0 effectively acts as an "unlimited" configuration (similarly to what type(uint256).max should achieve), and the limit can simply be re-configured to resolve the issue. As such, I believe it falls under vulnerabilities conditional on user mistakes and thus is of QA severity.

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-b