Skip to content

SECURITY: control.config.setUpstream is ordinary-tier, persists an attacker-chosen RPC upstream, and survives pairing.revoke #255

Description

@MichaelTaylor3d

Task

control.config.setUpstream sits at ordinary tier, persists a caller-chosen RPC upstream, and
survives pairing.revoke. By the rule dig-node-control-interface 0.18.0 just introduced, it belongs
at master tier.

Sibling of #254 (the 0.18.0 adoption), parent
https://github.com/DIG-Network/dig_ecosystem/issues/2870.

Measured on origin/main

  • Ordinary tier. is_pairing_admin_method (crates/dig-node-service/src/control.rs:281-286)
    matches only control.pairing.list|approve|revoke. setUpstream is absent, so a paired token
    reaches it — the same token class the crate documents as able to "drive control.* mutations but
    neither mint more tokens nor hide/revoke itself."
  • It persists. config_set_upstream (control.rs:898-920) validates only that the string carries no
    control character, normalizes it, then set_upstream_override(&ctx.config_path, &normalized) writes it
    to config.json.
  • It is read on next start as the RPC passthrough target — Config::from_env
    (crates/dig-node-service/src/config.rs:195-210): DIG_RPC_UPSTREAM env > the persisted override >
    the default.
  • pairing.revoke does not undo it. pairing::revoke_paired_token (pairing.rs:398-403) loads the
    paired-tokens file, retains away the id, writes it back. It touches no config value.

Why this is the same defect class as #254's item 1, with broader reach

The escalation delegates and is not revocable: after the call the attacker no longer needs the token,
and the operator's remedy — revoking the pairing — leaves the hostile value in place. That is precisely
why chiaPeers.add was moved to master tier rather than being handled in the remedy.

The reach is arguably wider. A trusted Chia peer bypasses corroboration for chain reads. The upstream
is where every method this node does not implement gets forwarded — dig-node ships with no upstream
precisely so that an unimplemented method answers a truthful local -32601 rather than something a third
party made up (config.rs:55-60 says exactly this, and says the emptiness must stay). Pointing it at an
attacker-controlled URL makes that surface answerable by the attacker.

One honest mitigation, which does not change the verdict: the running node captured its upstream at
construction, so the change takes effect on the next start (requires_restart). It is persistent and
silent rather than immediate — which makes it harder to notice, not less serious.

Scope

  1. Move control.config.setUpstream to master tier — by calling
    ControlMethod::requires_master_token()
    from the 0.18.0 contract, not by adding a string to
    is_pairing_admin_method. SECURITY: adopt dnci 0.18.0 — a paired token can still call chiaPeers.add here, because the master-tier predicate is restated as a string list #254 item 1 explains why: a predicate restated across a repo boundary as a
    string match is the byte-drift bug that produced this whole family.
  2. Decide and state whether any other ordinary-tier method persists caller-chosen state that survives
    revocation. The rule the contract now names is "master-tier means the effect outlives the token that
    invoked it."
    Apply it as a rule rather than patching two known instances — cache.setCap and
    log.setLevel also persist, and should be judged explicitly rather than left unexamined.
  3. Consider validating the upstream is a well-formed https URL. Today only control characters are
    rejected, and that check exists for a different reason (#526/B2 — a control character would be baked
    verbatim into a root-owned systemd unit line at install time).

Evidence

A test proving a paired token is refused on control.config.setUpstream and still accepted on the
ordinary reads it legitimately drives. Plus the lockstep test from #254 so dig-node's master set can never
silently diverge from the contract's again.

Effort

Medium for item 1, high for item 2 — item 2 is a judgement over the whole control surface, and it is the
half that stops this recurring.

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind:businessa person can DO something new, or money moves, or a shipped surface stops lying to them

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions