code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Whenever expressCaller calls expressReceiveTokenWithData, where token is a fee-on-transfer token, the amount parameter will be wrong. This could lead to wrong accounting in the destinationAddress.

Proof of Concept

Explanation of Functionality

Users can send tokens, along with a data payload to a destinationAddress on another chain via any of tokenManager's functions.

TokenManager calls InterchainTokenService#transmitSendToken to submit the call for Axelar gateway to execute.

execute is called on destination chain, which will call _processSendTokenWithDataPayload, which would

credit destinationAddress with amount of tokens sent
call executeWithInterchainToken on destinationAddress, passing as parameters sourceChain, sourceAddress, payload data attached by sender, tokenId, and the amount.

Here is the relevant part of _processSendTokenWithDataPayload:

function _processSendTokenWithDataPayload(string calldata sourceChain, bytes calldata payload) internal {
    ...
    {
        address expressCaller = _popExpressReceiveTokenWithData(
            tokenId,
            sourceChain,
            sourceAddress,
            destinationAddress,
            amount,
            data,
            commandId
        );
        if (expressCaller != address(0)) {
            amount = tokenManager.giveToken(expressCaller, amount);
            return;
        }
    }
    amount = tokenManager.giveToken(destinationAddress, amount);
    IInterchainTokenExpressExecutable(destinationAddress).executeWithInterchainToken(sourceChain, sourceAddress, data, tokenId, amount);
    ...
}

This function does it CORRECTLY. It uses giveToken to get the amount that would be passed as parameter to executeWithInterchainToken. giveToken returns the difference in balance of the destinationAddress after sending amount of tokens. So this would correctly handle fee-on-transfer tokens

The Problem

The Protocol also exposes a function called expressReceiveTokenWithData. This allows anyone to transfer amount tokens to the destination address, calls _expressExecuteWithInterchainTokenToken(which calls expressExecuteWithInterchainToken on destinationAddress), and makes the expressCaller the recipient when _processSendTokenWithDataPayload is called.

Here is expressReceiveTokenWithData function:

function expressReceiveTokenWithData(
        ...
) external {
    ...
    address caller = msg.sender;
    ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
    IERC20 token = IERC20(tokenManager.tokenAddress());

    SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);

    _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);

    _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
}

The PROBLEM is that this function does not handle fee-on-transfer tokens correctly. The expressCaller transfers amount tokens to destinationAddress(but destinationAddress receives only amount-x), and then, _expressExecuteWithInterchainTokenToken is called with amount as parameter(instead of amount-x).

Vulnerability Manifestation

Example 1

destinationAddress is a contract that updates sourceAddress balance by amount, and allows sourceAddress to withdraw its balance at anytime
Alice does an interchainTransfer of 100USDT
- Normally, if there is no expressCaller, _processSendTokenWithDataPayload will correctly call destinationAddress#executeWithInterchainToken with 99USDT(assuming fee is 1USDT), which would increment Alice balance by 99USDT
- Alice calls expressReceiveTokenWithData which would call destinationAddress#expressExecuteWithInterchainToken with 100USDT
- Alice balance is now 100USDT
Bob does an interchainTransfer of 100 USDT
- There is no expressCaller, so _processSendTokenWithDataPayload is called with 99USDT
- Bob balance is updated to 99USDT
Note that actual balance of contract is 99+99=198USDT
Alice withdraws 100USDT.
Bob can only withdraw 98USDT.

Example 2

destinationAddress is a contract that increments its totalBalance by amount, which will be withdrawable by Admin.
Alice does an interchainTransfer of 100USDT, and someone calls expressReceiveTokenWithData, which will update totalBalance to 100USDT(instead of 99USDT assuming fee is 1USDT)
Admin withdraw call will revert because it attempts to transfer totalBalance(100USDT) whereas actual balance is 99USDT

In general, all destinationAddresses that use amount in their accounting will be impacted

Tools Used

Manual Review

Recommended Mitigation Steps

Just as was done in _processSendTokenWithDataPayload(which uses giveToken to calculate amount which will be passed as parameter to external function call):

function _giveToken(address to, uint256 amount) internal override returns (uint256) {
    IERC20 token = IERC20(tokenAddress());
    uint256 balance = IERC20(token).balanceOf(to);

    SafeTokenTransferFrom.safeTransferFrom(token, liquidityPool(), to, amount);

    return IERC20(token).balanceOf(to) - balance;
}

,amount which will be passed as parameter to _expressExecuteWithInterchainTokenToken should be the difference in the balance of destinationAddress before and after safeTransferFrom is called:

function expressReceiveTokenWithData(
        ...
) external {
    ...
    address caller = msg.sender;
    ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenId));
    IERC20 token = IERC20(tokenManager.tokenAddress());
@>  uint256 balanceBefore = token.balanceOf(destinationAddress);
    SafeTokenTransferFrom.safeTransferFrom(token, caller, destinationAddress, amount);
@>  uint256 balanceAfter = token.balanceOf(destinationAddress);
@>  amount=balanceAfter-balanceBefore;
    _expressExecuteWithInterchainTokenToken(tokenId, destinationAddress, sourceChain, sourceAddress, data, amount);

    _setExpressReceiveTokenWithData(tokenId, sourceChain, sourceAddress, destinationAddress, amount, data, commandId, caller);
}