AxelarAmplifierGateway: Malicious signatures remain valid even when malicious signers were rotated out by operators

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/gateway/BaseAmplifierGateway.sol#L174-L188
https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/governance/BaseWeightedMultisig.sol#L125
https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/gateway/AxelarAmplifierGateway.sol#L99-L103

Vulnerability details

Details

The Amplifier Gateway has a mechanism to rotate message signers, that is, to change the current message signers to a new set of signers, the set themselves being signed-off by Axelar. To prevent a malicious signer set from hijacking the Amplifier Gateway, each rotation has an enforced delay.

During emergencies, e.g. when a signer set was determined to have been compromised, a trusted operator role can rotate to a new set of signers while bypassing the rotation delay. The emergency rotation itself requires signatures from Axelar, so said operator has to coordinate with the Axelar team to rotate out the compromised set of signers.

However, signatures that were made by the compromised signer set remains approved without methods to revoke the approval, and said messages will continue to remain valid even after the malicious signers were rotated out.

Vulnerability details

The Axelar Amplifier Gateway has a designated operator role that can bypass rotation delays:

bool enforceRotationDelay = msg.sender != _axelarAmplifierGatewayStorage().operator;
bool isLatestSigners = _validateProof(dataHash, proof);
if (enforceRotationDelay && !isLatestSigners) {
    revert NotLatestSigners(); // @audit rotation delay is only enforced if the sender is a non-operator
}

https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/gateway/AxelarAmplifierGateway.sol#L99-L103

This allows the operator (with a valid signature from Axelar) to rotate out a compromised signer set quickly, as per the NatSpec of said function.

When a signed message is sent from the Axelarnet Gateway to the Amplifier Gateway, the Weighted Multisig checks that the set of signers is sufficiently fresh, only then the message is approved

if (signerEpoch == 0 || currentEpoch - signerEpoch > previousSignersRetention) revert InvalidSigners(); // @audit prevents stale signers set from approving a message

https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/governance/BaseWeightedMultisig.sol#L125

However, once the message is approved, there is no further check during message validation. Any message that has been approved remains valid throughout its existence. There is also no functionality to invalidate any messages, nor to invalidate a set of signers alongside their messages.

https://github.com/code-423n4/2024-08-axelar-network/blob/main/axelar-gmp-sdk-solidity/contracts/gateway/BaseAmplifierGateway.sol#L174-L188

That means when a set of signers has been determined to be compromised and rotated out by the operators (alongside Axelar Governance), any malicious messages signed and approved remains valid without any methods to revoke the approval. This can compromise the security of e.g. timelocked executions or any messages that relies on permissioned/delayed executions as malicious messages will always be validate-able.

Impact

Any messages that were approved by a malicious signers set remain approved even when the malicious signer set were rotated out, without any methods to un-approve them. This can compromise the security of applications such that receiving messages are non-permissionless or requires a delay (e.g. timelocked executions).

Recommended mitigation

Similar to how operators can bypass the rotation cooldown given a designated signature from Axelar team, operators should also be able to cancel approved messages with another designated signature (or cancelling all messages from a given past epoch altogether).

Assessed type

Invalid Validation

[milapsheth]

The gateway operator exists to recover the gateway by rotating out a compromised signer set to an honest signer set, i.e the goal of this feature isn't to invalidate message approvals when the gateway was compromised but to avoid future compromised message approvals from being submitted. Invalidating message approvals is ineffective for the most part since they can be executed instantly (barring app-specific timelocks). Having said that, after the gateway is recovered, the gateway can still be upgraded to apply any custom migration (such as removing certain approvals) if needed. So, this report isn't really a concern.

[alex-ppg]

The Warden's submission details that a rotation of signers does not retroactively invalidate message approvals. I am inclined to agree with the Sponsor as it would be ineffective to do so due to their ability to be executed immediately (i.e. false sense of security) and consider this submission to be a QA (L) observation.

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-b

[gesha17]

@alex-ppg

I believe this is still a valid issue.

Replying to your comment:

The Warden's submission details that a rotation of signers does not retroactively invalidate message approvals. I am inclined to agree with the Sponsor as it would be ineffective to do so due to their ability to be executed immediately (i.e. false sense of security) and consider this submission to be a QA (L) observation.

It would be ineffective to do so if the honest signers have to submit transactions to invalidate the submitted malicios messages, but if they become invalid by design immediately after the malicios signers are rotated out, then its not ineffective. For example the validateContractCall() function can check if the signer set is still in rotation. The mitigation for this can be non-trivial, but this doesn't make the issue invalid.

---

Replying to the sponsors comment:

The gateway operator exists to recover the gateway by rotating out a compromised signer set to an honest signer set, i.e the goal of this feature isn't to invalidate message approvals when the gateway was compromised but to avoid future compromised message approvals from being submitted. Invalidating message approvals is ineffective for the most part since they can be executed instantly (barring app-specific timelocks). Having said that, after the gateway is recovered, the gateway can still be upgraded to apply any custom migration (such as removing certain approvals) if needed. So, this report isn't really a concern.

Upgrading the gateway to remove the malicios messages would be leaving the mitigation for after the fact. This would not handle any non-timelock messages that are still valid directly after the malicios signers are rotated out. For timelock messages it would depend on the duration of the timelock. If it's 5 weeks, then yes, maybe it is possible to apply this mitigation, but if it is 5 minutes, then it's obviously not.

---

To stress the importance of this, the contest page explicitly lists this as a place to look for bugs:

Does the message validation function accurately validate messages and update their status?

This seems to be directly violated, as the validation function incorrectly validates messages from signers that are rotated out.

Instead of this being a missing a function to invalidate messages, it can be viewed as a design flaw in which messages from malicios signers that have been rotated out are still executeable. Again, this can be non-trivial to fix, but it seems pretty important that malicios messages should not be executed, so I don't see how this can be a low severity...

[alex-ppg]

Hey @gesha17, thank you for your PJQA contribution!

It would be ineffective to do so if the honest signers have to submit transactions to invalidate the submitted malicios messages, but if they become invalid by design immediately after the malicios signers are rotated out, then its not ineffective. For example the validateContractCall() function can check if the signer set is still in rotation. The mitigation for this can be non-trivial, but this doesn't make the issue invalid.

The Sponsor meant that it is possible for a malicious signer to sign and immediately execute a transaction in a single payload. This means that it would be impossible for a rotation to be executed in time before a harmful action is done.

Upgrading the gateway to remove the malicios messages would be leaving the mitigation for after the fact. This would not handle any non-timelock messages that are still valid directly after the malicios signers are rotated out. For timelock messages it would depend on the duration of the timelock. If it's 5 weeks, then yes, maybe it is possible to apply this mitigation, but if it is 5 minutes, then it's obviously not.

Upgrading a contract is never considered an acceptable mitigation, and this submission was evaluated on its original C4-compliant merits.

This seems to be directly violated, as the validation function incorrectly validates messages from signers that are rotated out.

Instead of this being a missing a function to invalidate messages, it can be viewed as a design flaw in which messages from malicios signers that have been rotated out are still executeable. Again, this can be non-trivial to fix, but it seems pretty important that malicios messages should not be executed, so I don't see how this can be a low severity...

To "fix" this vulnerability, we would have to remove features that the Axelar Network protocol desires. Given that the incidence of a malicious signer is considered low and an adequate rescue mechanism already exists, I cannot consider any alternatives to the system currently in place as HM vulnerabilities. To re-iterate, a malicious signature may be capitalized on the moment it is created so even if we invalidate signatures after a rotation the harm would have already been done.