code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L47

Vulnerability details

Impact

This is not a bug in the code but more of a logical and architectural issue. A lot of recent bridges exploits were due to some stolen private keys that could be used to sign messages and steal the protocol funds. This is why it is so important to keep our message verifying process decentralized so it will not be possible to hack the system with only a couple of stolen keys. The protocol tries to implement such solution with the need of a majority of operators to sign a transaction on the bridge. The problem is the there is an owner address that can set those operator to what ever he wants, so basically the owner is a point of failure of the protocol and in a case of a stolen owner key the protocol can be hacked and lose all of its money. A lot of other bridges demand the majority of the operators to change the operators addresses, so in that way the protocol stays decentralized (see in PoC an example of such implementation in other system).

Proof of Concept

As we can see here: https://github.com/code-423n4/2022-07-axelar/blob/main/contracts/auth/AxelarAuthWeighted.sol#L47 The owner can call transferOperatorship and set the operators to whatever he wants, for example a single address that is in his conrol. And after that he has full control over the protocol and can steal all the protocol funds.

And as we can see here this is other implementation of setting the operators with the need of a majority of validators to do so. https://github.com/crypto-org-chain/gravity-bridge/blob/v2.0.0-cronos/solidity/contracts/Gravity.sol#L249

Tools Used

Vscode.

Recommended Mitigation Steps

I recommend to find a way to set the operators in a decentralized way. Perhaps consider the solution I show on PoC.

GalloDaSballo commented 2 years ago

Admin privilege is historically Medium Severity

Report seems to lack actual POC (only shows transfer of ownership)

re1ro commented 2 years ago

In our deployment setup the owner of the AxelarAuthWeighted is the AxelarGateway itself. So transferOperatorship can be called only by the gateway contract and transferOperatorship command needs to be signed by the majority of operators first.

GalloDaSballo commented 2 years ago

The warden makes a statement about the owner being a center vulnerability point for AxelarAuthWeighted

The sponsor disputes by stating that such contract will be owned by the AxerarGateway meaning that operators will be effectively acting as a multisig for the contract.

I think the statement from the Warden has validity in that the systems weakest point seems to be the transferOperatorship function, however there's a difference between tating that that's the weak link to proving it as such and showing how to break it.

Because of that I think that the finding can be "interpreted" as owner is able to change weights and kick operators in one block.

Because the Network itself relies on trusted validators, I think a more appropriate severity for this report is QA

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