possibility of DOS when calling `setFlowLimit` function

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L534-L541 https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/token-manager/TokenManager.sol#L172 https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/utils/FlowLimit.sol#L34-L39

Vulnerability details

Impact

there is possibility of dos happen when the function setFlowLimit is called because this function will loop throw all tokenIDs and then it will call each tokenID tokenManager.setFlowLimit to set the flow limit, the possible dos can happen because the FlowLimit.sol#_setFlowLimit have SSTORE key which store the value in specific slot. if the specific slot is empty and this is the first time the operators called this function to add flow limit for all tokenIDs then each SSTORE call will cause 20000 of gas unit according to the EVM opcodes and this may cause DOS, Writing to a previously empty storage slot costs a significant amount of gas:

https://www.evm.codes/#55?fork=shanghai

Proof of Concept

the function setFlowLimit will call the setFlowLimit in each tokenID's token manger contract in a for loop :

function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
    uint256 length = tokenIds.length;
    if (length != flowLimits.length) revert LengthMismatch();
    for (uint256 i; i < length; ++i) {
        //@audit dos may happen because
        ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenIds[i]));
        tokenManager.setFlowLimit(flowLimits[i]);
    }
}

the tokenManger seFlowLimit function which it make call to the _setFlowLimit function in flowLimit.sol contract :

function setFlowLimit(uint256 flowLimit) external onlyOperator {
    _setFlowLimit(flowLimit);
}

_setFlowLimitfunction inflowLimit.sol

function _setFlowLimit(uint256 flowLimit) internal {
    assembly {
        //audit it cost 20,000 gas for empty slot
        sstore(FLOW_LIMIT_SLOT, flowLimit)
    }
    emit FlowLimitSet(flowLimit);
}

as we can see if a user or attacker calling the setFlowLimit for the first time ever which the flow limit slot is empty with long length array then the system will face dos or out-of-gas.

Tools Used

manual review

Recommended Mitigation Steps

recommend to add for loop limit to set limit for making loop or at least add limit for the first call ever to set the flow limit, adding minimum Limit for a for loop may help a lot and this mechanism the chainlink CCIP was using it to avoid DOS to happen.

Assessed type

DoS

[0xSorryNotSorry]

It would be nice to see a coded POC reverting the function due to gas shortage.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[berndartmueller]

tokenIds supplied to the setFlowLimit function is controlled by the caller (i.e., the operator) and thus can limit the number of token IDs to provide, preventing the issue from arising.

[c4-judge]

berndartmueller marked the issue as unsatisfactory: Invalid