code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/interchain-governance-executor/InterchainProposalSender.sol#L59

Vulnerability details

Impact

In InterchainProposalSender, function sendProposals reverts due to exceeding gas limit.

The bug arises from unpredictable gas estimation during transaction execution, leading to transaction reverts. The block gas limit is 30000000, but the contract's gas estimation failed to accurately predict the required gas for the call to sendProposals, causing the transaction to fail, creating a DOS state.

Proof of Concept

describe('sendProposals', function () {
        it.only('should return eth if it reverts on sendProposals', async function () {
            const target = await ethers.getSigners().then((signers) => signers[1].getAddress());
            const valueToSend = ethers.utils.parseEther('2');
            await signer.sendTransaction({
                to: sender.address,
                value: valueToSend,
            });
            const calls = [
                {
                    target,
                    value: 0,
                    callData: ethers.utils.randomBytes(32),
                },
                {
                    target,
                    value: 1,
                    callData: ethers.utils.randomBytes(32),
                },
                {
                    target,
                    value: 2,
                    callData: ethers.utils.randomBytes(32),
                },
            ];

            const destChains = ['avalanche', 'binance'];

            const xCalls = [
                {
                    destinationChain: destChains[0],
                    destinationContract: ethers.constants.AddressZero,
                    gas: 1,
                    calls,
                },
                {
                    destinationChain: destChains[1],
                    destinationContract: ethers.constants.AddressZero,
                    gas: 1,
                    calls,
                },
            ];

            const payload = ethers.utils.defaultAbiCoder.encode(
                ['address', 'tuple(address target, uint256 value, bytes callData)[]'],
                [signerAddress, calls],
            );

            const initialSenderBalance = await hre.ethers.provider.getBalance(sender.address);
            console.log('Initial Sender Balance:', initialSenderBalance.toString());

                // block gas limit 30000000
            for (const destChain of destChains) {
                const tx = await sender.sendProposals(xCalls, { value: 2, gasLimit: 30000000});
                const receipt = await tx.wait();
                console.log('Initial Sender Balance:', initialSenderBalance.toString());

                if (receipt.status === 0) {
                    // Transaction reverted, so the sender should receive the refunded ETH
                    const finalSenderBalance = await hre.ethers.provider.getBalance(sender.address);
                    console.log('Final Sender Balance after revert:', finalSenderBalance.toString());
                    console.log('Are sender balances equal after revert?', finalSenderBalance.eq(initialSenderBalance.add(2)));

                    // Ensure that the sender received the returned ETH after the revert
                    expect(finalSenderBalance.toNumber()).to.equal(initialSenderBalance.add(2).toNumber());

                    return; // Exit the test as it behaved as expected
                } else {
                    // Transaction did not revert, so the sender should not receive the returned ETH
                    const finalSenderBalance = await hre.ethers.provider.getBalance(sender.address);
                    console.log('Final Sender Balance in iteration:', finalSenderBalance.toString());
                    console.log('Are sender balances not equal after iteration?', !finalSenderBalance.eq(initialSenderBalance.add(2)));

                    // Ensure that the function does not revert in this iteration
                    expect(finalSenderBalance.toNumber()).to.not.equal(initialSenderBalance.add(2).toNumber());

                    // Ensure that the sender did not receive the returned ETH
                    const ethReceived = await hre.ethers.provider.getBalance(sender.address);
                    console.log('ETH Received:', ethReceived.toString());
                    expect(ethReceived.toNumber()).to.equal(initialSenderBalance.toNumber());
                }
            }
        });
    });
});

Tools Used

Manual Review

Recommended Mitigation Steps

I would recommend deep testing to ascertain the gas limit required for the function to execute successfully across all chains and then setting the gas limit to the maximum required for each desired chain.