Skip to content

security: manifest path fields allow path traversal / arbitrary file write on the host running dincli #227

Closed

Description

@Abidoyesimze

Summary

Every dincli command that fetches a model-owner-supplied service file or artifact builds the destination path by joining a trusted local base directory with a raw path string taken directly from the model's manifest JSON, then passes it to ensure_file_exists, which creates parent directories and writes the fetched content with no containment check. Since manifest content is never validated on-chain, a malicious model owner can make any client/auditor/aggregator who interacts with their model overwrite an arbitrary file on their own machine.

Where

dincli/cli/context.py:490-540 (ensure_file_exists):

file_path.parent.mkdir(parents=True, exist_ok=True)
retrieve_from_ipfs(ipfs_cid, file_path)

No path validation anywhere in this function.

Vulnerable call sites (all build base_dir / Path(manifest["path"]) or equivalent, verified against develop @ 740a613):

  • dincli/cli/context.py ~L316 — task-contract ABI artifact path (task_contracts.<key>.artifact.path) — pure file-write, no code execution needed to trigger
  • dincli/cli/client.py ~L129-134 (train-lms)
  • dincli/cli/aggregator.py ~L336-341, ~L557-562
  • dincli/cli/auditor.py ~L450-462
  • dincli/cli/modelownerd/{gi,aggregation,auditor_batches,model}.py — model-owner-side equivalents

The only path-side defense found is dincli/services/ipfs.py's _normalize_path:

def _normalize_path(path: str | Path) -> Path:
    safe_path = Path(path).expanduser().resolve()
    dangerous_roots = (Path("/etc"), Path("/boot"), Path("/dev"), Path("/proc"))
    if any(str(safe_path).startswith(str(root)) for root in dangerous_roots):
        logger.warning(f"Reading or writing through a sensitive system path: {safe_path}")
    return safe_path

This only logs a warning for four hardcoded system directories and never blocks the write or checks containment against the trusted base directory — everything else (~/.ssh/authorized_keys, ~/.bashrc, crontabs, any other user-writable path) passes through silently. Python's Path.__truediv__ also discards the base directory entirely if the manifest's path is an absolute string:

>>> Path('/home/victim/.cache/dincli/model_5') / Path('/home/victim/.ssh/authorized_keys')
PosixPath('/home/victim/.ssh/authorized_keys')

foundry/src/DINModelRegistry.sol's requestModelRegistration/approveModel only validate slasher status and task-contract ownership — the manifest's bytes32 CID is stored and never dereferenced/validated on-chain, so manifest content (every path field included) is entirely attacker-controlled by whoever owns that model.

This bypasses, rather than overlaps with, the one sandboxing control the codebase already has for model-owner-supplied content: dincli/cli/worker.py runs custom service code inside a Docker container. But the vulnerable ensure_file_exists downloads happen in the host CLI process, before run_worker_container is ever invoked — so this write lands on the bare host regardless of that mitigation.

Exploit

  1. Attacker deploys their own DINTaskCoordinator/DINTaskAuditor, gets them authorized as slashers (the normal model-owner flow), registers and gets approved — none of this inspects manifest content.
  2. Attacker publishes a manifest containing, e.g.:
    {
      "type": "custom",
      "train_client_model": {
        "path": "/home/victim/.ssh/authorized_keys",
        "ipfs": "<CID of attacker's SSH public key>"
      }
    }
  3. A validator runs the documented command: dincli client train-lms <attacker_model_id>.
  4. client_service_path = model_base_dir / Path(manifest["path"]) resolves to /home/victim/.ssh/authorized_keys, discarding the cache base dir.
  5. ensure_file_exists creates ~/.ssh if needed, downloads, and silently overwrites authorized_keys with the attacker's key — persistent SSH access to the victim's machine, independent of any later Docker-sandboxed execution.

The same pattern applies via the aggregator.py/auditor.py/modelownerd/*.py call sites for their respective roles.

Recommendation

Resolve the manifest-supplied path against the trusted base directory and reject (don't silently re-anchor) anything not contained within it, e.g.:

resolved = (base_dir / manifest_path).resolve()
if not resolved.is_relative_to(base_dir.resolve()):
    raise ValueError(f"manifest path escapes base directory: {manifest_path}")

Apply this once, centrally, inside ensure_file_exists (or a shared path-construction helper every call site is required to go through) rather than patching each of the ~8 call sites individually.

Activity

  1. umeradl commented on Oct 6, 2026

    @umeradl
    Member

    Status update: confirmed; scheduled as task_061026_22 Part B

    Checked against develop @ a1fcce2. The finding holds:

    • DinContext.ensure_file_exists (dincli/cli/context.py:490) creates parent directories and writes whatever IPFS returns to the path it's given, with no containment check.
    • Every manifest-driven call site builds that path as <base_dir> / Path(manifest[...]["path"]):
      • client.py:129-130
      • aggregator.py:336-337, :557-558
      • auditor.py:450-453
      • modelownerd/aggregation.py:399-400, modelownerd/auditor_batches.py:144
      • modelownerd/model.py:103-104, :219-220
      • the task-contract artifact path at context.py:316 (model_base_path / artifact_rel_path)
    • An absolute path replaces the base directory outright, and .. segments walk out of it.
    • dincli/services/ipfs.py:81 _normalize_path only logs a warning for four system roots, and never blocks the write.
    • The writes happen in the host CLI process, before any Docker worker runs, so the worker sandbox doesn't help.

    Can be done now: yes. It goes in task_061026_22 as Part B, assigned to you. Notes for the fix:

    • One shared helper, e.g. resolve_manifest_path(base_dir, manifest_path) -> Path. It rejects absolute paths and anything whose resolve() isn't inside base_dir.resolve() (Path.is_relative_to), raising rather than silently re-anchoring.
    • Every call site goes through it. ensure_file_exists also checks containment, as a backstop for future call sites.
    • Symlinks: decide how a pre-existing symlink inside the cache dir is handled (resolve before the check), and say so in the PR.
    • Tests: pytest in the existing monkeypatch / CliRunner style covering an absolute path, ../ traversal, a symlink escape, and a normal relative path that still works, for at least one call site per role plus the artifact path.

    Coordination: task-plan-051026-2 (PR No. 229) TP-3 also edits auditor.py and context.py for new commands. Whichever lands second rebases. Full spec: task_061026_22 (linked from the tracking discussion once it's open).

  2. umeradl commented on Oct 8, 2026

    @umeradl
    Member

    Done by PR No. 237 (task_061026_22 Part B). It was merged into develop as 0be3d4a, with a review fix in 920c93e. Outcome comment; the develop push CI is green. I'm closing this by hand, because a merge into develop doesn't trigger closing keywords (main is the default branch).

    Checked on origin/develop @ 920c93e:

    • B1, shared helper: resolve_manifest_path is at dincli/cli/utils.py:46.

      • It rejects an absolute path outright (:58).
      • It rejects anything whose (base_dir / path).resolve() isn't is_relative_to(base_dir.resolve()) (:62-65).
      • It raises ManifestPathEscapesBaseError, naming the manifest key, and never re-anchors.
      • resolve() follows symlinks, so a symlink inside the base that points outside is rejected. That's documented in the docstring and the PR.
    • B2, every call site: all 19 manifest-path joins now go through the helper:

      • client ×2
      • aggregator ×4 (T1 and T2)
      • auditor ×3
      • modelownerd/ aggregation ×2, auditor_batches ×1 and model ×4
      • the task-contract artifact path (context.py:319), which falls back to the bundled ABI on an escape
      • modelownerd/gi.py ×2, a site the PR found with the suggested git grep sweep

      git grep '\["path"\]' dincli leaves only:

      • the four worker job-spec "service_path" entries. The host already validated those same strings, and the container reads them under a :ro mount.
      • system.py:1418, which reads the bundled, repo-shipped cid_services.json rather than a manifest.
    • B3, backstop: DinContext.ensure_file_exists checks that the target resolves inside base_dir before any IPFS fetch (context.py:539-542). _normalize_path's warning is left in place.

      • Deviation: the PR's CACHE_DIR default for base_dir broke the model owner's local task flow (create-genesis-model, submit-genesis-model, distribute-mnist). 920c93e makes base_dir required on both ensure_file_exists and load_custom_fn, scoped to the role:

        • get_model_base_dir(model_id) = CACHE_DIR/<net>/model_<id> for clients, auditors, aggregators and the registered model owner
        • get_task_dir(coordinator) = cwd/tasks/<net>/<coord> for the model owner before registration

        That's the "CACHE_DIR and the task directory" option from the task spec, made strict.

    • B4, tests: tests/test_manifest_path_traversal.py has 16 tests:

      • helper cases: relative, absolute, absolute that looks like a path inside the base, .., symlink escape, and . == base
      • the backstop: outside path rejected, missing base_dir is a TypeError, no writing into another model's dir
      • one call site per role (client train_lms, aggregator aggregate_t1, auditor evaluate_lms, model owner create_testdataset) plus the artifact path, each raising before any download
      • a regression test for the cwd task flow

      The PR description shows break-then-fix on client.py.

    • Deliverables: pytest -m "not integration" gives 468 passed and 0 failed, with green CI on 920c93e. Documentation/public/manifest.md's path row says the path must be relative and stay inside the model's cache or task directory.

    No out-of-scope follow-ups were raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions