code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/utils/Multicall.sol#L27-L29

Vulnerability details

Description

Multicall can be used to do multiple calls to InterchainTokenService in one transaction. There is however a mistake when a call reverts:

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/its/utils/Multicall.sol#L27-L29

File: its/utils/Multicall.sol

27:            if (!success) {
28:                revert(string(result));
29:            }

The first 4 bytes of the revert result is the signature, like Error(string) (0x08c379a), similar to a function selector. Then follows the abi encoded string of the error. Simply casting this to a string will result in a string decode error.

Further reading here: https://ethereum.stackexchange.com/questions/83528/how-can-i-get-the-revert-reason-of-a-call-in-solidity-so-that-i-can-use-it-in-th

Impact

Reverts in Multicall will not be forwarded correctly not providing the user with the correct feedback.

As the code is clearly intended to forward the revert reason I believe this to be medium.

Proof of Concept

Test in Multicall in utils.js:

    it('should forward revert reason', async () => {
        const revertFunctionData =  (await test.populateTransaction.reverter()).data;
        // this fails with a string decoding error
        await expect(test.multicall([revertFunctionData])).to.be.reverted.revertedWith("Ceci n'est pas une revert");
    });

With this addition to its/test/MulticallTest.sol:

    function reverter() external pure {
        revert("Ceci n'est pas une revert");
    }

Tools Used

Manual audit

Recommended Mitigation Steps

Consider taking the implementation from uniswap (which originally is from https://ethereum.stackexchange.com/a/83577): https://github.com/Uniswap/v3-periphery/blob/main/contracts/base/Multicall.sol#L16-L23

16:            if (!success) {
17:                // Next 5 lines from https://ethereum.stackexchange.com/a/83577
18:                if (result.length < 68) revert();
19:                assembly {
20:                    result := add(result, 0x04)
21:                }
22:                revert(abi.decode(result, (string)));
23:            }