VH-Lab / VH-Lab/DID-matlab

Optional ingest-time gating for untrusted document sources

Open
#172 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
MATLAB
Stars
1
Forks
2
Avg merge
2h 18m
Merged PRs (30d)
39

Description

Context

Follow-up to #167 and #169. After #169 (PR #171), validateIngestFileEntry refuses only unsafe uid values — the ingest source location is trusted verbatim, on the reasoning that the caller who says "add this file to my DB" chose the source. This unblocks legitimate ingest workflows (NDI-matlab stages sources under /tmp/nditemp/ and adds them to a DB whose .ndi lives elsewhere) and keeps the destination fully constrained under <FileDir>/<uid> via isSafeUid.

The read-side isSafeLocalLocation filter (do_open_doc line 767) still defends orig_location, so a crafted stored location cannot steer open_doc to read outside db_dir through the orig_location branch.

The gap

There is one code path that ingestion isn't gated against, and it only matters when the document JSON is attacker-controlled (a cloud pull, not a locally authored document):

  • A document with location='../../etc/passwd' (or any traversal / absolute-outside path pointing at a readable file) now ingests successfully on POSIX. copyfile resolves the traversal, reads the target file, and writes its contents into <FileDir>/<uid>.
  • On a later open_doc, the read-side filter refuses orig_location, but by then the cache candidate at <FileDir>/<uid> — built from the safe uid — already exists and wins the earlier loop; the smuggled contents are served back.

There is no such issue when the source of the document is trusted (the local NDI-matlab call, say). It is specifically the cloud-pull path where the JSON is not the caller's.

What we're not doing

For now, no change. The trade-off from #169 stands: reintroducing a source-side containment check breaks the legitimate workflow, and this residual surface only matters for the "attacker-authored document" case — one downstream packages can also mitigate by not blindly ingesting cloud-pulled documents.

What an optional gate might look like

If we do decide to close this later:

  • Narrow the guard to relative locations with a traversal segment (containsTraversal(location) && !isAbsolutePath(location) after rebasing against db_dir) — this refuses the ../../etc/passwd shape without refusing an absolute path that legitimately lives outside db_dir. It's stricter than "no traversal", weaker than "must be inside db_dir".
  • Or make the guard opt-in via an add_docs(..., 'trustSources', true|false) name-value, defaulting to true (today's behavior) and set to false by callers who pull documents from a cloud store. Downstream (NDI cloud pull) then sets trustSources=false and gets the strict containment check.

Either would need matching changes on the Python side and a bridge sync note. See the DID-python companion issue for the parity version.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with validateIngestFileEntry, the do_open_doc filter near line 767, and the add_docs entry point; review the proposed containsTraversal/isAbsolutePath behavior and the Python companion issue. No implementation is currently selected—the issue says no change for now—so completion would require an agreed gate design, matching Python changes, and a bridge sync note.

Written by the indexing model from the issue text.

Assessment

Tech stack
matlab, python
Domain
databases, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.