mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-27 00:36:39 +00:00
harden(bash): stop .. source paths escaping the corpus tree (#2596)
The #2690 form-3 (`$(dirname …)`) branch had no `..` guard and the tracked form-4 branch let `..` walk past the base to an arbitrary host path, so a hostile corpus could make the extractor stat and record an out-of-tree file (source "$(dirname "$VAR")/../../../../etc/passwd") — a corpus-side info leak, since resolve_bash_source_edges only filters *resolved* cross-file edges, not the extractor's own edge/probe. Adds a lexical _within_tree gate: form 3 rejects `..` outright (its base is a guess), and a tracked form-4 base may reach a sibling via $VAR/../lib but cannot escape past base.parent. Adds two traversal-rejection tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
b345dc4993
commit
f52b8dbc69
@@ -7,6 +7,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
- Fix: a cross-file INFERRED `uses` edge now binds to the symbol whose body actually references the imported name (a module-level function is a valid source; a co-located class that never touches the import gets no edge), instead of fanning out from the import line to every class in the importing file (#2652, thanks @ousamabenyounes). A reference at module top level, with no enclosing symbol, emits no edge.
|
||||
- Fix: a wiki article link now targets the article's filename verbatim instead of a percent-encoded twin, so a label with `( ) & #` or non-ASCII characters no longer produces a link that names no file on disk; the link and the on-disk filename share one canonicalization (#2597, thanks @abhay-codes07).
|
||||
- Fix: export filenames are now budgeted against the full destination path rather than only the per-component `NAME_MAX`, so a long output directory on Windows no longer pushes an Obsidian/wiki note path past `MAX_PATH` and aborts the export mid-write (#2655, thanks @abhay-codes07). The collision-suffix reserve was widened so a four-digit dedup suffix can't overrun the budget.
|
||||
- Fix: the Bash extractor resolves two more `source` path forms — `source "$(dirname "$VAR")/lib/x.sh"` and a `..` suffix on a tracked-variable base (`source "$VAR/../lib/x.sh"`) — so those cross-file source edges are no longer silently dropped (#2596, thanks @hudsonwa). A `..` on a *guessed* base is still rejected, and a tracked-base `..` cannot walk past the base's parent to an arbitrary host path, so a hostile corpus can't make the extractor stat or record an out-of-tree file.
|
||||
- Feature: OCaml `.ml`/`.mli` extraction via tree-sitter-ocaml (optional `[ocaml]` extra). Extracts modules, top-level and module-level values/functions, types and their variant constructors, `open` imports, and function calls; qualified calls (`Geo.area`) resolve to the value, and cross-file `open`/call targets collapse onto the unique real definition via the corpus stub rewire.
|
||||
- Fix: a JS/TS `for...of` / `for...in` loop binding is now shadowed, so passing it as a call argument no longer fabricates an `indirect_call` edge to an unrelated same-named callable (#2685, thanks @ousamabenyounes); completes the loop/closure/catch shadow family (#2568/#2569/#2517).
|
||||
- Fix: graph provenance (`built_at_commit`) is stamped from the analysed repository rather than the shell's working directory, so `graphify extract` run from elsewhere records the target's commit, not the caller's (#2534 family; #2699, thanks @C0KERNEL).
|
||||
|
||||
@@ -37,6 +37,24 @@ def _bash_source_suffix(raw: str, allow_dotdot: bool = False) -> str | None:
|
||||
return suffix
|
||||
|
||||
|
||||
def _within_tree(ceiling: Path, target: Path) -> bool:
|
||||
"""True if *target* is *ceiling* or lives beneath it, compared lexically
|
||||
(normpath, no filesystem access).
|
||||
|
||||
A ``source`` path built with ``..`` (the ``$VAR/../lib`` idiom, #2596 form 4,
|
||||
or a ``$(dirname …)`` prefix) must not be allowed to walk up to an arbitrary
|
||||
host path: a corpus is attacker-controllable, and both the ``is_file()``
|
||||
existence probe and the recorded absolute ``target_file`` on the emitted edge
|
||||
are a corpus-side information leak (``source "$VAR/../../../../etc/passwd"``).
|
||||
Callers gate the probe/emit on this so a target that escapes the allowed tree
|
||||
is dropped before it is ever stat-ed. Defense in depth only:
|
||||
``resolve_bash_source_edges`` independently keeps a *resolved* edge only when
|
||||
the target is itself a scanned corpus file."""
|
||||
c = os.path.normpath(str(ceiling))
|
||||
t = os.path.normpath(str(target))
|
||||
return t == c or t.startswith(c + os.sep)
|
||||
|
||||
|
||||
# Recognise ``$(dirname "$VAR")`` (or ``$(dirname "${VAR}")``) at the start of
|
||||
# a ``source`` argument, capturing the variable name so the source resolver
|
||||
# can treat the whole construct as ``var_bases[VAR].parent`` (#2596 form 3).
|
||||
@@ -334,7 +352,13 @@ def extract_bash(path: Path) -> dict:
|
||||
# Strip the $(dirname ...) prefix and any leading
|
||||
# slash to get the literal suffix.
|
||||
suffix = raw[dirname_match.end():].lstrip("/")
|
||||
if suffix and "$" not in suffix:
|
||||
# The base is a guessed script dir, so reject a
|
||||
# `..` suffix outright — same policy as
|
||||
# _bash_source_suffix(allow_dotdot=False); without
|
||||
# it, `$(dirname "$VAR")/../../../etc/passwd`
|
||||
# resolves and gets probed/recorded (#2596).
|
||||
if (suffix and "$" not in suffix
|
||||
and ".." not in suffix.split("/")):
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
@@ -369,9 +393,18 @@ def extract_bash(path: Path) -> dict:
|
||||
base = var_bases[var_name]
|
||||
if var_match and var_name in var_bases:
|
||||
resolved = Path(os.path.normpath(base / suffix))
|
||||
# A tracked base may reach a sibling via
|
||||
# `$VAR/../lib`, so the ceiling is one
|
||||
# level up — but `..` must not walk past
|
||||
# it to an arbitrary host path (#2596).
|
||||
ceiling = base.parent
|
||||
else:
|
||||
resolved = (base / suffix).resolve()
|
||||
if resolved.is_file():
|
||||
# Untracked base: `..` was already rejected
|
||||
# by _bash_source_suffix, so the target is
|
||||
# under base; the gate is belt-and-braces.
|
||||
ceiling = base
|
||||
if _within_tree(ceiling, resolved) and resolved.is_file():
|
||||
add_edge(file_nid, _make_id(str(resolved)),
|
||||
"imports_from", line,
|
||||
confidence="INFERRED", context="import",
|
||||
|
||||
@@ -2817,6 +2817,51 @@ def test_extract_bash_source_dotdot_suffix_script_dir_guess_still_rejected(tmp_p
|
||||
assert not targets, f"untracked var with .. should not resolve: {targets}"
|
||||
|
||||
|
||||
def test_extract_bash_source_dirname_cmdsubst_rejects_traversal(tmp_path):
|
||||
"""Form 3 hardening (#2596): the `$(dirname …)` base is a guessed directory,
|
||||
so a `..` suffix must be rejected before the target is probed or recorded —
|
||||
otherwise `source "$(dirname "$VAR")/../../../../etc/passwd"` resolves to an
|
||||
arbitrary host path and leaks it as a source edge on an attacker-controlled
|
||||
corpus."""
|
||||
outside = tmp_path / "secret.sh"
|
||||
outside.write_text("echo secret\n", encoding="utf-8") # a real file to escape to
|
||||
script = tmp_path / "proj" / "bin" / "x.sh"
|
||||
script.parent.mkdir(parents=True)
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"\n'
|
||||
'source "$(dirname "$SCRIPT_DIR")/../../secret.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
assert not result["bash_sources"], result["bash_sources"]
|
||||
leaked = [e.get("target_file") for e in result["edges"]
|
||||
if e.get("relation") == "imports_from" and "secret.sh" in (e.get("target_file") or "")]
|
||||
assert not leaked, f"traversal target leaked as an edge: {leaked}"
|
||||
|
||||
|
||||
def test_extract_bash_source_dotdot_tracked_var_cannot_escape_to_root(tmp_path):
|
||||
"""Form 4 hardening (#2596): a tracked base legitimately reaches a sibling via
|
||||
`$VAR/../lib`, but a multi-level `..` that walks past the base's parent to an
|
||||
arbitrary host path must be dropped, not probed and recorded."""
|
||||
outside = tmp_path / "secret.sh"
|
||||
outside.write_text("echo secret\n", encoding="utf-8")
|
||||
# bin is two levels below tmp_path, so ../../.. escapes past base.parent.
|
||||
script = tmp_path / "proj" / "bin" / "x.sh"
|
||||
script.parent.mkdir(parents=True)
|
||||
script.write_text(
|
||||
'#!/usr/bin/env bash\n'
|
||||
'SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"\n'
|
||||
'source "$SCRIPT_DIR/../../../secret.sh"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
result = extract_bash(script)
|
||||
assert not result["bash_sources"], result["bash_sources"]
|
||||
leaked = [e.get("target_file") for e in result["edges"]
|
||||
if e.get("relation") == "imports_from" and "secret.sh" in (e.get("target_file") or "")]
|
||||
assert not leaked, f"traversal target leaked as an edge: {leaked}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# JSON extractor tests (#866)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user