code-423n4 / 2024-02-hydradx-findings

1 stars 0 forks source link

Unrestricted Privilege Allows Constraint Bypass #105

Closed c4-bot-8 closed 7 months ago

c4-bot-8 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L324-L356 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L324-L356 https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/circuit-breaker/src/lib.rs#L324-L356

Vulnerability details

Description

The process for enabling and disabling the HydraDX Circuit Breaker rely on privileged origins and roles: HydraDX-node/pallets/circuit-breaker/src/lib.rs So the potential risks would revolve around permissioning controls of the TechnicalOrigin

Impact

The Circuit Breaker relies on a privileged TechnicalOrigin to set critical limits. However the robustness of this root-of-trust is unclear, risking arbitrary control or constraint bypass.

The issue originates from the limit setting access control: HydraDX-node/pallets/circuit-breaker/src

Granting total governance of constraints to the opaque TechnicalOrigin. With no further decentralization, review, or emergency processes around this role.

Impact

Such compromise could completely bypass circuit breaker assumptions - enabling instability, market manipulation, and severe loss events.

This breaks the security model preventing arbitrary privilege escalation.

Proof of Concept

The elevated permission model indicates a strong reliance on the benevolence of the Technical Origin. Economic and engagement assumptions may degrade without decentralized protections or oversight.

Specifically, reliance on the privileged TechnicalOrigin is encoded here: HydraDX-node/pallets/circuit-breaker/src/lib.rs

pub fn set_trade_volume_limit(
  origin: OriginFor<T>,
  ...
) -> DispatchResult {

  T::TechnicalOrigin::ensure_origin(origin)?;

  ...

}

This ensure_origin check grants the TechnicalOrigin full control over setting limits without any further oversight or decentralization.

A weakness emerges in that this origin becomes a central point of failure - if compromised or poorly managed, it could undermine Circuit Breaker protections.

So while the logic meets specifications, supplemental controls around the trusted root TechnicalOrigin role may be warranted.

Simulation Steps

  1. Snapshot valid baseline state

  2. Compromise the TechnicalOrigin account

    • Adjust limits to extremely high thresholds
  3. Execute attacks now permissible

    • Arbitrarily drain liquidity
    • Distort markets via massive swaps
  4. Monitor breach of valid economic assumptions

    • Pool instability
    • Severe loss events
  5. Quantify impacts

    • Loss assessments
    • Collateral damage analyses

Outcome

This attack path helps quantify the impacts of centralization within the Circuit Breaker protections - including potential privilege escalation vectors.

Tools Used

Manual Review

Recommended Mitigation Steps

Some ways to improve this could be:

Assessed type

Governance

c4-pre-sort commented 7 months ago

0xRobocop marked the issue as insufficient quality report

c4-pre-sort commented 7 months ago

0xRobocop marked the issue as duplicate of #108

c4-judge commented 7 months ago

OpenCoreCH marked the issue as unsatisfactory: Invalid