diff --git a/CHANGELOG.md b/CHANGELOG.md index 065e0c8b..a8d6bb98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/graphify/extractors/bash.py b/graphify/extractors/bash.py index f1d5e419..3c387be1 100644 --- a/graphify/extractors/bash.py +++ b/graphify/extractors/bash.py @@ -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", diff --git a/tests/test_extract.py b/tests/test_extract.py index e30a0811..0b894f9f 100644 --- a/tests/test_extract.py +++ b/tests/test_extract.py @@ -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) # ---------------------------------------------------------------------------