Deployer wallet retains ability to spoof validated senders after ownership transfer

mediumCode4rena

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/9f642fe854eb11ad9f18fe028e5a8353c258e926/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L72-L74 https://github.com/code-423n4/2023-07-axelar/blob/9f642fe854eb11ad9f18fe028e5a8353c258e926/contracts/its/remote-address-validator/RemoteAddressValidator.sol#L83

Vulnerability details

Impact

The InterchainTokenService is deployed using create3: https://github.com/code-423n4/2023-07-axelar/blob/9f642fe854eb11ad9f18fe028e5a8353c258e926/scripts/deploy.js#L60-L64 - this means that its address depends on the address of the deployer wallet, the address of the Create3Deployer contract, and the salt that is extracted from a deployment key.

It should be feasible, then, for the same deployer wallet to deploy a Create3Deployer with the same address on a new chain, and use the same deployment key to deploy a contract with the same address as the InterchainTokenService but arbitrary implementation. A team member has confirmed on Discord that "the address will be the interchainTokenServiceAddress which is constant across EVM chains.". However, at deployment time, only a subset of all the possible EVM chains will be supported, and more may be added in the future. When that happens, it is possible that the original deployer wallet is compromised or no longer available.

On the other hand, the addTrustedAddress function validates that the sender is the owner of the RemoteAddressValidator contract. This owner is originally the deployer wallet, but ownership may be transferred (and it would be a good practice to transfer it to a more secure governance multisig immediately after deployment). After ownership transfer, the previous owner should not be allowed to add trusted addresses.

However, the validateSender function will treat any address that is the same as the current chain's InterchainTokenService as valid. A malicious contract deployed by the deployer wallet after the ownership transfer could be treated as valid and would have the ability to deploy token managers and standardized tokens, or perform arbitrary token transfers. This is a form of centralization risk but is also a serious privilege escalation, as it should be possible to strip the deployer of the ability to perform these actions, and this gives them virtually unlimited power even after an ownership transfer.

Proof of Concept

• A deployer account is used to deploy all contracts on chain A.
• Ownership of the RemoteAddressValidator and all other contracts is transferred to governance multisigs. After this point, the deployer should have no ability to add trusted addresses.
• The deployer account is compromised (which is easier to do than compromising a governance multisig), or a team member with access to the deployer account becomes compromised or malicious.
• The deployer repeats the same steps used for the deployment, starting from the same wallet nonce, on chain B, but replaces the InterchainTokenService contract with a contract that allows arbitrary messages, e.g. by adding this function to a regular InterchainTokenService:

    function callContract(
        string calldata destinationChain,
        bytes memory payload,
        uint256 gasValue,
        address refundTo
    ) external onlyOwner {
        _callContract(destinationChain, payload, gasValue, refundTo);
    }

• The deployer then uses this contract to send a payload with SELECTOR_SEND_TOKEN and using the deployer address as destination to the InterchainTokenService on chain A.
• When running validateSender on the RemoteAddressValidator on chain A, this check will pass and the address will be treated as valid:

        if (sourceAddressHash == interchainTokenServiceAddressHash) {
            return true;
        }

• Therefore, the tokens will be transferred to the deployer address on chain A.

Tools Used

Visual inspection.

Recommended Mitigation Steps

The assumption that the InterchainTokenService address is valid in any chain is dangerous, because of how the contract is created and the possibility that new EVM chains may exist in the future. A deployer EOA should not have this amount of permission for an indefinite time. I would recommend breaking that assumption and requiring that all addresses are added as trusted addresses explicitly.

A check can therefore be added to only treat the interchain token service's address as valid if the source chain is also the same chain where the RemoteAddressValidator is deployed. The following diff shows a way to do this:

diff📋
diff --git a/contracts/its/remote-address-validator/RemoteAddressValidator.sol b/contracts/its/remote-address-validator/RemoteAddressValidator.sol
index bb101e5..c2e5382 100644
--- a/contracts/its/remote-address-validator/RemoteAddressValidator.sol
+++ b/contracts/its/remote-address-validator/RemoteAddressValidator.sol
@@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
 import { IRemoteAddressValidator } from '../interfaces/IRemoteAddressValidator.sol';
 import { AddressToString } from '../../gmp-sdk/util/AddressString.sol';
 import { Upgradable } from '../../gmp-sdk/upgradable/Upgradable.sol';
+import { StringToBytes32 } from '../../gmp-sdk/util/Bytes32String.sol';
 
 /**
  * @title RemoteAddressValidator
@@ -11,23 +12,25 @@ import { Upgradable } from '../../gmp-sdk/upgradable/Upgradable.sol';
  */
 contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
     using AddressToString for address;
+    using StringToBytes32 for string;
 
     mapping(string => bytes32) public remoteAddressHashes;
     mapping(string => string) public remoteAddresses;
     address public immutable interchainTokenServiceAddress;
     bytes32 public immutable interchainTokenServiceAddressHash;
     mapping(string => bool) public supportedByGateway;
-
+    bytes32 public immutable currentChainName;
     bytes32 private constant CONTRACT_ID = keccak256('remote-address-validator');
 
     /**
      * @dev Constructs the RemoteAddressValidator contract, both array parameters must be equal in length
      * @param _interchainTokenServiceAddress Address of the interchain token service
      */
-    constructor(address _interchainTokenServiceAddress) {
+    constructor(address _interchainTokenServiceAddress, string memory _chainName) {
         if (_interchainTokenServiceAddress == address(0)) revert ZeroAddress();
         interchainTokenServiceAddress = _interchainTokenServiceAddress;
         interchainTokenServiceAddressHash = keccak256(bytes(_lowerCase(interchainTokenServiceAddress.toString())));
+        currentChainName = _chainName.toBytes32();
     }
 
     /**
@@ -69,7 +72,7 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
     function validateSender(string calldata sourceChain, string calldata sourceAddress) external view returns (bool) {
         string memory sourceAddressLC = _lowerCase(sourceAddress);
         bytes32 sourceAddressHash = keccak256(bytes(sourceAddressLC));
-        if (sourceAddressHash == interchainTokenServiceAddressHash) {
+        if (sourceAddressHash == interchainTokenServiceAddressHash && sourceChain.toBytes32() == currentChainName) {
             return true;
         }
         return sourceAddressHash == remoteAddressHashes[sourceChain];

Assessed type

Access Control