Malicious Authority can break key functionalities

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/pallets/omnipool/src/lib.rs#L157 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/pallets/stableswap/src/lib.rs#L143

Vulnerability details

Bug Description

In the HydraDX protocol, specific functions are reserved for execution by the AuthorityOrigin, a role not defined as a trusted actor within the contest's context. This implies the potential for malicious actions by this entity. There are multiple functions that can be called by the AuthorityOrigin.

Stbaleswap:

• create_pool()
• update_pool_fee()
• update_amplification()
• set_asset_tradable_state()

Omnipool:

• add_token()
• refund_refused_asset()
• remove_token()
• withdraw_protocol_liquidity()

Using these functions, a malicious authority can break key functionalities of the protocol. This could for example be by setting the pool fee to 100% making users lose 100% to fees, or by refunding refused assets to itself.

Impact

A malicious authority can break some of the key functionalities of the protocol.

Proof of Concept

The possibility for the Authority to call all of these functions can be seen when looking at the functions inside Stableswap/Omnipool and checking which of them are only callable by the authority, which is indicated by the T::AuthorityOrigin::ensure_origin(origin)?; line.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate these risks, implementing a MultiSig mechanism for the AuthorityOrigin role is advisable. This approach would ensure that multiple trusted parties must agree before any significant changes are enacted, considerably reducing the likelihood of malicious actions. Additionally, the protocol should establish clear guidelines and oversight mechanisms for actions taken by the AuthorityOrigin to safeguard against potential abuse and maintain the protocol's operational security and user trust.

Assessed type

Governance

[c4-pre-sort]

0xRobocop marked the issue as insufficient quality report

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[c4-sponsor]

enthusiastmartin (sponsor) disputed

[enthusiastmartin]

It wasIt is already multisig.

It was already mentioned in the context description that such report wont be considered valid.

[OpenCoreCH]

Centralization risk, which would be QA in any case. However, this was explicitly out of scope:

Omnipool has AuthorityOrigin parameter which allows only configured origin to perform certain actions. That means reports such as if origin is not configured correctly, it can lead to ... are not valid.

[c4-judge]

OpenCoreCH marked the issue as unsatisfactory: Out of scope