code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/interchain-token-service/InterchainTokenService.sol#L530-L549

Vulnerability details

Impact

README.md (Scope->FlowLimit.sol) states that flow limits "act as a security measure to minimize damage of exploits". It is also written (Scope->Operatable.sol) that the Axelar Service Governance contract will be the operator of the Interchain Token Service, and naturally, in InterchainTokenService.sol, the function setFlowLimits is restricted to onlyOperator.

However, when I asked the sponsor on Discord who the owner of InterchainTokenService.sol will be, dean.axelar#1908 responded that the README.md is in fact incorrect. He stated that the owner of the InterchainTokenService.sol will be the AxelarServiceGovernance, and the operator will be privately controlled Axelar team Multisig.

As a result, Axelar Governance will be unable to call setFlowLimits, preventing AXL owners from reacting to exploits. This is a dangerous centralization of power because only the main Axelar team will be able to activate this protective measure. Since this issue may indirectly result in loss of funds, as the ability to react to fund-draining exploits will be compromised, this issue is of Medium severity.

Note that even if the operator of InterchainTokenService.sol was the Axelar Governance, they would then not be able to call setPaused, as it is restricted to onlyOwner. This contradicts the README again (Scope->Pausable.sol): "This contract provides a mechanism to halt the execution of specific functions on the interchain token service if a paused by the governance operator."

This strongly indicates that, regardless of who is intended to be the Owner and/or the Operator of InterchainTokenService.sol, there is clearly confusion within the Axelar team regarding who will have access to these critical control functions in InterchainTokenService.sol.

Tools Used

VSCode

Recommended Mitigation Steps

As setFlowLimits and setPaused are critical control functions in InterchainTokenService.sol and are meant to mitigate exploits, they should both be callable by Axelar Governance. Whether the Axelar Governance is the operator or the owner of InterchainTokenService.sol is a matter of semantics; Axelar Governance just needs to be given access to these functions.

If the Axelar Governance is meant to be the owner of InterchainTokenService.sol, the following will fix the issue:

index 2a13bdf..f8bd9fb 100644
--- a/contracts/its/interchain-token-service/InterchainTokenService.sol
+++ b/contracts/its/interchain-token-service/InterchainTokenService.sol
@@ -531,7 +531,7 @@ contract InterchainTokenService is
      * @param tokenIds an array of the token Ids of the tokenManagers to set the flow limit of.
      * @param flowLimits the flowLimits to set
      */
-    function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOperator {
+    function setFlowLimit(bytes32[] calldata tokenIds, uint256[] calldata flowLimits) external onlyOwner {
         uint256 length = tokenIds.length;
         if (length != flowLimits.length) revert LengthMismatch();