Axelar Network - nobody2018's results

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/interchain-token-service/InterchainTokenService.sol#L484-L486

Vulnerability details

Impact

InterchainTokenService.expressReceiveTokenWithData does not apply the Check-Effect-Interaction pattern. In some edge cases, the caller can lose funds.

Proof of Concept

The life cycle of sending token across chains contains 3 phases:

//P1: launch on source chain
TokenManager.callContractWithInterchainToken
  interchainTokenService.transmitSendToken
    interchainTokenService._callContract
      gateway.callContract
        emit ContractCall
        
//P2: The off-chain program collects all events of the target chain, 
//builds the Command array, and operators sign for them. 
//Call AxelarGateway.execute of the target chain to set each command to be executed to true.
AxelarGateway.execute
  IAxelarAuth(AUTH_MODULE).validateProof
  //Loop through setting each command to true
  _setCommandExecuted(commandId, true);
  _setCommandExecuted(commandId, true);
  ......
  
//P3: The relayer calls InterchainTokenService.execute for each command, 
//or it can be called by the user himself.
InterchainTokenService.execute  //we only consider SELECTOR_SEND_TOKEN_WITH_DATA here.
  gateway.validateContractCall
  InterchainTokenService._execute
    _processSendTokenWithDataPayload
      _popExpressReceiveTokenWithData	
      tokenManager.giveToken
      destinationAddress.executeWithInterchainToken

expressReceiveTokenWithData uses the caller's tokens to fullfill a callContractWithInterchainToken ahead of time. It is only called before the command's execution status was set to true, due to this check. That is, before the completion of P2 in the above life cycle. It first transfers the token from the caller to destinationAddress, then calls destinationAddress.expressExecuteWithInterchainToken, and finally calls _setExpressReceiveTokenWithData to set the value in the corresponding slot to the caller address(called expressCaller). Such a process is actually the Check-Interaction-Effect mode. destinationAddress can be a malicious contract, and it is very dangerous to interact with the malicious contract before setting the slot.

Suppose the following scenario:

1. Bob deploys a malicious contract A and calls TokenManager.callContractWithInterchainToken to send 100e18 token.

2. Alice's program periodically (every 1 minute) checks whether there is an outgoing sendToken. No such events were detected this time.

3. When the off-chain program calls AxelarGateway.execute, this tx1 enters the memory pool, pending.

4. Alice's program checks again. This time the command initiated by contract A in step 1 is detected. Therefore, call InterchainTokenService.expressReceiveTokenWithData. tx2 enters the mempool.

5. Bob notices the parameter input of tx1. Call A.setParram to submit this parameter to contract A (higher gas). tx3 enters the memory pool.

6. tx3 is prioritized due to higher gas payment.

7. When tx2 is executed, expressReceiveTokenWithData first transfers the token of 100e18 from alice to A, and then calls A.expressExecuteWithInterchainToken, where there are 2 parts.

   • AxelarGateway.execute(input), this input is obtained in step 5. This is to set all commands contained in input as executable.
   • InterchainTokenService.execute executes the command initiated in step 1. Since expressCaller has not been set at this time, 100e18 token is sent to A here.

   Execution flow returns here, where expressCaller is set to alice. However, alice will never be able to obtain the 100e18 token. A gets 200e18 tokens.

8. tx1 is executed without affecting the result.

In summary, A gets an extra 100e18, which belongs to alice. This scenario is not common. It's an edge case.

Tools Used

Manual Review

Recommended Mitigation Steps

File: contracts\its\interchain-token-service\InterchainTokenService.sol
467:     function expressReceiveTokenWithData(
468:         bytes32 tokenId,
469:         string memory sourceChain,
470:         bytes memory sourceAddress,
471:         address destinationAddress,
472:         uint256 amount,
473:         bytes calldata data,
474:         bytes32 commandId
475:     ) external {
476:         if (gateway.isCommandExecuted(commandId)) revert AlreadyExecuted(commandId);
477: 
478:         address caller = msg.sender;
479:         ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
480:         IERC20 token = IERC20(tokenManager.tokenAddress());
481: 
482:         SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);
483:         
484:-        _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);
484:+        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
485: 
486:-        _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
486:+        _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);
487:     }

Assessed type

Reentrancy

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/token-manager/TokenManager.sol#L83-L88 https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/token-manager/TokenManager.sol#L109-L114 https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/token-manager/TokenManager.sol#L136-L142

Vulnerability details

Impact

Anyone can deploy a TokenManagerLockUnlock for the existing ERC20 via registerCanonicalToken, and deploy the corresponding token on a target chain via deployRemoteCanonicalToken. After the TokenManagers of the two chains are created, users can transfer token between the two chains via sendToken/callContractWithInterchainTokenfer. If the existing ERC20 is ERC777, such as imBTC, TokenManager will suffer reentrancy attack. This will allow the attacker to steal token from TokenManager.

Proof of Concept

Let's take a look at the _transfer function of ERC777:

function _transfer(address recipient, uint256 amount) internal returns (bool) {
        require(recipient != address(0), "ERC777: transfer to the zero address");

        address from = msg.sender;
        //@audit callback to from
        _callTokensToSend(from, from, recipient, amount, "", "");

        _move(from, from, recipient, amount, "", "");
        //@audit callback to recipient
        _callTokensReceived(from, from, recipient, amount, "", "", false);

        return true;
    }

sendToken/callContractWithInterchainToken/transmitInterchainTransfer call _takeToken(sender, amount) internally, and overwrite amount with the return value, which is used as a parameter for interchainTokenService.transmitSendToken.

File: contracts\its\token-manager\implementations\TokenManagerLockUnlock.sol
44:     function _takeToken(address from, uint256 amount) internal override returns (uint256) {
45:         IERC20 token = IERC20(tokenAddress());
46:         uint256 balance = token.balanceOf(address(this));
47: 
48:         SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), amount);//@audit callback to from
49: 
50:         // Note: This allows support for fee-on-transfer tokens
51:         return IERC20(token).balanceOf(address(this)) - balance;
52:     }

If from is a contract that implements IERC777Sender.tokensToSend, the execution flow will callback to from. from can call sendToken/callContractWithInterchainToken again. The attack flow is as following:

//assume that token=imBTC 
//token.balanceOf(address(this))=10e8
//token.balanceOf(address(from))=2e8

TokenManager.sendToken   //amount=1e8
  _takeToken
    uint256 balance = 10e8;
    SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), 1e8);
      from.tokensToSend
        TokenManager.sendToken   //amount=1e8
          _takeToken
        uint256 balance = 10e8;
        SafeTokenTransferFrom.safeTransferFrom(token, from, address(this), 1e8);
          from.tokensToSend
        return IERC20(token).balanceOf(address(this)) - balance; //11e8 - 10e8 = 1e8
      interchainTokenService.transmitSendToken //@@@@@emit event, amount=1e8
    return IERC20(token).balanceOf(address(this)) - balance; //12e8 - 10e8 = 2e8
  interchainTokenService.transmitSendToken  //@@@@@emit event, amount=2e8

From the above flow, we can see that interchainTokenService.transmitSendToken is called twice. The amount of the first call is 1e8, and the amount of the second call is 2e8. The total amount sent to the target chain is 3e8, but the amount of token held by from is only 2e8. This opens up the opportunity for an attacker to drain the tokens of the TokenManager.

Tools Used

Manual Review

Recommended Mitigation Steps

Add reentrancy protection for sendToken/callContractWithInterchainToken/transmitInterchainTransfer.

Assessed type

Reentrancy

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/token-manager/TokenManager.sol#L161-L165

Vulnerability details

Impact

InterchainTokenService._processSendTokenPayload/_processSendTokenWithDataPayload is used to process token sending commands. Both will eventually call tokenManager.giveToken to send the token to the receiver. If the token is ERC777, the giveToken may be manipulated by the receiver, which can make the subsequent giveToken revert from other users. This will consume the relayer's gas, which is paid by the user in the source chain.

Proof of Concept

For existing ERC20, TokenManager can be TokenManagerLockUnlock or TokenManagerLiquidityPool. TokenManager.giveToken first calls _giveToken from the child contract to send the token to the receiver, and then calls _addFlowIn to add a flow in amount.

File: contracts\its\token-manager\TokenManager.sol
161:     function giveToken(address destinationAddress, uint256 amount) external onlyService returns (uint256) {
162:         amount = _giveToken(destinationAddress, amount);
163:         _addFlowIn(amount);
164:         return amount;
165:     }

TokenManagerLockUnlock._giveToken and TokenManagerLiquidityPool._giveToken have similar logic. They use the AfterBalance(to)-BeforeBalance(to) mode to return the amount of token transferred. This method is no problem for erc20 without callback. But if the token is ERC777 and to is a malicious contract, the return value of _giveToken can be manipulated.

File: contracts\its\token-manager\implementations\TokenManagerLockUnlock.sol
60:     function _giveToken(address to, uint256 amount) internal override returns (uint256) {
61:         IERC20 token = IERC20(tokenAddress());
62:         uint256 balance = IERC20(token).balanceOf(to);
63: 
64:         SafeTokenTransfer.safeTransfer(token, to, amount);//@audit if token is ERC777, there is callback to 'to' inside this function
65: 
66:         return IERC20(token).balanceOf(to) - balance;
67:     }

The attack flow is as follows:

TokenManager.giveToken	//amount=1e18
  TokenManagerLockUnlock._giveToken
    balance = IERC20(token).balanceOf(to);	//assuming that to's balance is 0.
    SafeTokenTransfer.safeTransfer(token, to, amount);
      to.tokensReceived
        token.transterFrom(EOA, address(this), 100000e18); //EOA is wallet from attacker.
    return IERC20(token).balanceOf(to) - balance; //return value is 100001e18.
   _addFlowIn(amount)	//here amount is 100001e18.

If the current flow limit is not 0, then if the amount returned by _giveToken just meets this condition, that is, flowToAdd + flowAmount is close to flowToCompare + flowLimit. flowAmount is the amount returned by _giveToken. In this way, the subsequent _addFlowIn will revert because the condition is not met.

However, _addFlowIn internally reads FlowInSlot and FlowOutSlot by epoch, and the interval between each epoch is EPOCH_TIME (six hours). 4 epochs a day. The attacker can send tokens (very small amount) across chains every 6 hours without paying gas, then the relayer will not call such a transaction. In this way, when a new epoch arrives, the attacker calls InterchainTokenService.execute to trigger _processSendTokenPayload. Make subsequent calls to revert by attack flow mentioned above.

Tools Used

Manual Review

Recommended Mitigation Steps

For TokenManagerLockUnlock:

File: contracts\its\token-manager\implementations\TokenManagerLockUnlock.sol
60:     function _giveToken(address to, uint256 amount) internal override returns (uint256) {
61:         IERC20 token = IERC20(tokenAddress());
62:-        uint256 balance = IERC20(token).balanceOf(to);
62:+	    uint256 balance = IERC20(token).balanceOf(address(this));
63: 
64:         SafeTokenTransfer.safeTransfer(token, to, amount);
65: 
66:-        return IERC20(token).balanceOf(to) - balance;
66:+        return balance - IERC20(token).balanceOf(address(this);
67:     }

For TokenManagerLiquidityPool:

File: contracts\its\token-manager\implementations\TokenManagerLiquidityPool.sol
094:     function _giveToken(address to, uint256 amount) internal override returns (uint256) {
095:         IERC20 token = IERC20(tokenAddress());
096:-        uint256 balance = IERC20(token).balanceOf(to);
096:+        uint256 balance = IERC20(token).balanceOf(liquidityPool());
097: 
098:         SafeTokenTransferFrom.safeTransferFrom(token, liquidityPool(), to, amount);
099: 
100:-        return IERC20(token).balanceOf(to) - balance;
100:+        return balance - IERC20(token).balanceOf(liquidityPool());
101:     }

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L76

Vulnerability details

Impact

An proposal will be finally executed from InterchainProposalExecutor contract on the destination chain. A proposal contains an array of InterchainCalls.Call, where each Call structure includes call information. If Call.value is greater than 0, it means that the native token needs to be sent when executing the call. However, InterchainProposalExecutor.execute/executeWithToken (from the parent class AxelarExecutable) is not payable and do not implement the payable fallback function. Therefore, executing such a proposal will only revert.

Proof of Concept

When a proposal is executed in the target chain, the flow is as follows:

AxelarExecutable.execute
  InterchainProposalExecutor._execute
    InterchainProposalExecutor._executeProposal
      //loop InterchainCalls.Call[]
      call.target.call
      call.target.call
      ...

Let's look at the code snippet of execute:

File: contracts\gmp-sdk\executable\AxelarExecutable.sol
17:     function execute(
18:         bytes32 commandId,
19:         string calldata sourceChain,
20:         string calldata sourceAddress,
21:         bytes calldata payload
22:     ) external {	//@audit  no payable
23:         bytes32 payloadHash = keccak256(payload);
24: 
25:         if (!gateway.validateContractCall(commandId, sourceChain, sourceAddress, payloadHash)) revert NotApprovedByGateway();
26: 
27:         _execute(sourceChain, sourceAddress, payload);
28:     }

Then look at the code of _executeProposal:

File: contracts\interchain-governance-executor\InterchainProposalExecutor.sol
73:     function _executeProposal(InterchainCalls.Call[] memory calls) internal {
74:         for (uint256 i = 0; i < calls.length; i++) {
75:             InterchainCalls.Call memory call = calls[i];
76:             (bool success, bytes memory result) = call.target.call{ value: call.value }(call.callData);
77: 
78:             if (!success) {
79:                 _onTargetExecutionFailed(call, result);//@audit If the contract does not have enough value, it will revert inside this function.
80:             } else {
81:                 _onTargetExecuted(call, result);
82:             }
83:         }
84:     }

Finally, the implementation of function receive()/fallback() payable was not found in InterchainProposalExecutor and the parent class AxelarExecutable.

Therefore, we can conclude that if a proposal needs to send native token to the target contract, then the proposal will never succeed.

Tools Used

Manual Review

Recommended Mitigation Steps

Two fixes:

• Add the payable keyword to InterchainProposalExecutor.execute/executeWithToken.
• Add function receive() external payable to the InterchainProposalExecutor contract.

Assessed type

Payable