Proposal could remain executable after cancellation on destination

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/gmp-sdk/util/TimeLock.sol#L67

Vulnerability details

Impact

Axelar Interchain governance allows them to initiate action on the source and execute them on the destination after timelock. Two of these actions include schedule and cancellation. Axelar can schedule a update on the destination by sending cross chain message from the source and then either wait for finalization or cancel it. However, there is no check inside cancel to check if the scheduled proposal exists or not.

    function _cancelTimeLock(bytes32 hash) internal {
        if (hash == 0) revert InvalidTimeLockHash();

        _setTimeLockEta(hash, 0);
    }

Hence if there is a case where Axelar scheduled an update on the source, and before it's committed on the destination, chose to cancel it, someone can execute cancel first on the destination and keep the proposal active. This could have serious impacts depending on the nature of the update

Proof of Concept

Consider Chain A as the source, and Chain B as a destination

1. Chain A: Msg 1: Send Cross Chain message for scheduling update
2. Chain B: The scheduling update is yet to be committed by Axelar network validators or relayers
3. Chain A: Msg 2: Axelar notices a mistake in the update and chooses to cancel the update, and sends cross chain message to B for the same
4. Chain B: Both Msg 1 and 2 are committed by Axelar network validators and are available for execution
5. Chain B: Now the problem is neither Axelar Gateway enforces that order should be 1 -> 2, nor the timelock enforces that cancel (2) can only happen after schedule (1). Hence someone can execute 2 and later 1, keeping an invalid update proposal active for execution.

Tools Used

NA

Recommended Mitigation Steps

Consider adding a check to enforce proposal existence inside _cancelTimeLock

Assessed type

Invalid Validation

[0xSorryNotSorry]

someone can execute cancel first on the destination and keep the proposal active.

There are many points off in this submission like how someone will call an internal function to cancel the proposal? OR what effect will there be if the validation of the hash is done since non-existing hashes can't be cancelled anyway.

Judge perusal required.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[berndartmueller]

The PoC is very vague with many assumptions involving miss-operation by the governance.

However, the submission correctly points out that the order in which messages are executed is not enforced. As soon as a message gets approved (via the AxelarGateway's SELECTOR_APPROVE_CONTRACT_CALL), anyone can execute the message with AxelarExecutable.execute, even out of order.

Kindly invite the sponsor's feedback before deciding on the severity and validity of this submission. @deanamiel

[deanamiel]

This submission is not a valid vulnerability. It is the responsibility of governance to ensure that a proposal actually exists before attempting to cancel it. That is true, anyone can call the external function AxelarExecutable.execute, but the internal execute function in the governance contracts is protected by the onlyGovernance modifier which will cause the call to revert if the sender is not Axelar governance.

[c4-sponsor]

deanamiel (sponsor) disputed

[berndartmueller]

This submission is not a valid vulnerability. It is the responsibility of governance to ensure that a proposal actually exists before attempting to cancel it. That is true, anyone can call the external function AxelarExecutable.execute, but the internal execute function in the governance contracts is protected by the onlyGovernance modifier which will cause the call to revert if the sender is not Axelar governance.

Agree! The burden is on the governance to ensure the correct operation in regard to proposals. Downgrading to QA (Low).

[c4-judge]

berndartmueller changed the severity to QA (Quality Assurance)

[c4-judge]

berndartmueller marked the issue as grade-c