mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-15 00:19:33 +00:00
security(fortran): strip #include directives before cpp (GHSA-pcc4-rvhr-2pr8)
The Fortran capital-F cpp step relied on -nostdinc -I /dev/null to block #include-based host-file reads, but cpp always searches the source file's own directory for quoted includes and always honours absolute paths regardless of those flags. A malicious .F/.F90 file with #include "/etc/passwd" or #include "../../../secret" could inline arbitrary host files into graph.json/GRAPH_REPORT.md and the LLM context on the default offline path (CWE-22/73/200, high severity). Strip every #include directive before preprocessing and feed the sanitized source to cpp on stdin, so cpp never opens the file's directory or any absolute/traversing include path. Macro expansion (#define/#ifdef, the actual reason capital-F files need cpp) is preserved; cross-file include fidelity is traded away to close the arbitrary-read vector. Feeding stdin also removes the -I/etc/x.F90 argument-injection edge case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
c66eebe25f
commit
e1d9deed6a
2 files changed
+129
-58
No files matched your search
@@ -2,38 +2,64 @@
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
from graphify.extractors.base import _file_stem, _make_id, _read_text
|
||||
|
||||
|
||||
_FORTRAN_CPP_EXTS = {".F", ".F90", ".F95", ".F03", ".F08"}
|
||||
|
||||
# A cpp `#include` directive at the start of a (optionally indented) line, in
|
||||
# either the `"..."` or `<...>` form. Matched on raw bytes before preprocessing.
|
||||
_INCLUDE_DIRECTIVE_RE = re.compile(rb"^[ \t]*#[ \t]*include\b")
|
||||
|
||||
|
||||
def _strip_include_directives(source: bytes) -> bytes:
|
||||
"""Blank every cpp `#include` directive, preserving line numbers.
|
||||
|
||||
cpp always searches the directory of the file being preprocessed for quoted
|
||||
includes and always honours absolute include paths, regardless of
|
||||
`-nostdinc`/`-I` — so a malicious capital-F Fortran source containing
|
||||
`#include "/etc/passwd"` or `#include "../../../secret"` would inline
|
||||
arbitrary host files into the graph we ship to the LLM and persist to disk
|
||||
(GHSA-pcc4-rvhr-2pr8, CWE-22/73/200). The `-nostdinc -I /dev/null` mitigation
|
||||
did not cover those two cases. Removing the directives entirely keeps macro
|
||||
expansion (the reason capital-F files need cpp — `#ifdef MPI`, `#define
|
||||
REAL8`) while making host-file reads impossible. Cross-file include fidelity
|
||||
is lost, which is an acceptable trade for closing an arbitrary-read vector on
|
||||
the default offline path. Each stripped line is blanked (not deleted) so
|
||||
reported line numbers still match the original source.
|
||||
"""
|
||||
return b"\n".join(
|
||||
b"" if _INCLUDE_DIRECTIVE_RE.match(line) else line
|
||||
for line in source.split(b"\n")
|
||||
)
|
||||
|
||||
|
||||
def _cpp_preprocess(path: Path) -> bytes:
|
||||
"""Run cpp -w -P on a capital-F Fortran file and return preprocessed bytes.
|
||||
|
||||
Falls back to raw file bytes if cpp is not available. Capital-F extensions
|
||||
conventionally require C preprocessor expansion (#ifdef MPI, #define REAL8, etc.)
|
||||
before parsing.
|
||||
Falls back to (include-stripped) raw file bytes if cpp is not available.
|
||||
Capital-F extensions conventionally require C preprocessor expansion
|
||||
(#ifdef MPI, #define REAL8, etc.) before parsing.
|
||||
|
||||
Security (F-007): we pass `-nostdinc` and `-I /dev/null` so a malicious
|
||||
source file containing `#include "/home/victim/.ssh/id_rsa"` (or any other
|
||||
include directive) cannot inline arbitrary host files into the output that
|
||||
we then ship to an LLM. Without these flags `cpp` happily resolves any
|
||||
relative or absolute include path it can read, which is a corpus-side
|
||||
file-exfiltration vector.
|
||||
Security (GHSA-pcc4-rvhr-2pr8): every `#include` directive is stripped before
|
||||
preprocessing (see `_strip_include_directives`) and the sanitized source is
|
||||
fed to cpp on stdin, so cpp never opens the source file's directory or any
|
||||
absolute/traversing include path — closing the corpus-side arbitrary
|
||||
file-read/exfiltration vector that `-nostdinc -I /dev/null` alone left open.
|
||||
Feeding stdin (rather than a file path) also removes the `-I/etc/x.F90`
|
||||
argument-injection edge case entirely.
|
||||
"""
|
||||
import shutil
|
||||
import subprocess
|
||||
safe = _strip_include_directives(path.read_bytes())
|
||||
if not shutil.which("cpp"):
|
||||
return path.read_bytes()
|
||||
return safe
|
||||
try:
|
||||
# Pass an absolute path so a corpus file named like "-I/etc/x.F90" cannot
|
||||
# be parsed by cpp as an option (cpp does not accept a "--" end-of-options
|
||||
# terminator). What matters is that an absolute path cannot begin with
|
||||
# "-" — not that it begins with "/", which only holds on POSIX (a Windows
|
||||
# absolute path starts with a drive letter, and is equally safe).
|
||||
result = subprocess.run(
|
||||
["cpp", "-w", "-P", "-nostdinc", "-I", "/dev/null", str(path.resolve())],
|
||||
["cpp", "-w", "-P", "-nostdinc", "-I", "/dev/null"],
|
||||
input=safe,
|
||||
capture_output=True,
|
||||
timeout=30,
|
||||
)
|
||||
@@ -41,7 +67,7 @@ def _cpp_preprocess(path: Path) -> bytes:
|
||||
return result.stdout
|
||||
except Exception:
|
||||
pass
|
||||
return path.read_bytes()
|
||||
return safe
|
||||
|
||||
def extract_fortran(path: Path) -> dict:
|
||||
"""Extract programs, modules, subroutines, functions, use statements, and calls from Fortran files.
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
"""The Fortran C-preprocessor path is hardened against argument injection (F5).
|
||||
"""The Fortran C-preprocessor path is hardened against corpus-side file reads.
|
||||
|
||||
A corpus file is attacker-named; cpp does not accept a "--" end-of-options
|
||||
terminator, so _cpp_preprocess passes an absolute path which can never be parsed
|
||||
as a cpp option.
|
||||
A capital-F Fortran source is attacker-controlled. cpp always searches the
|
||||
source file's own directory for quoted `#include`s and always honours absolute
|
||||
include paths, regardless of `-nostdinc`/`-I`, so `#include "/etc/passwd"` or
|
||||
`#include "../../secret"` would inline arbitrary host files into the graph
|
||||
(GHSA-pcc4-rvhr-2pr8). `_cpp_preprocess` now strips every `#include` directive
|
||||
before preprocessing and feeds the sanitized source to cpp on stdin, so no host
|
||||
file can be read and no attacker-named path is ever passed as a cpp argument.
|
||||
"""
|
||||
import os
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -12,11 +16,12 @@ import pytest
|
||||
from graphify import extract
|
||||
|
||||
|
||||
def _capture_cpp_argv(monkeypatch):
|
||||
def _capture_cpp_call(monkeypatch):
|
||||
captured = {}
|
||||
|
||||
def fake_run(argv, **kwargs):
|
||||
captured["argv"] = argv
|
||||
captured["input"] = kwargs.get("input")
|
||||
|
||||
class _Result:
|
||||
returncode = 0
|
||||
@@ -29,43 +34,83 @@ def _capture_cpp_argv(monkeypatch):
|
||||
return captured
|
||||
|
||||
|
||||
def test_cpp_preprocess_passes_absolute_path(tmp_path, monkeypatch):
|
||||
f = tmp_path / "weird.F90"
|
||||
def test_cpp_preprocess_feeds_stdin_and_passes_no_file_path(tmp_path, monkeypatch):
|
||||
"""cpp is invoked with only flags; the source arrives on stdin, so an
|
||||
attacker-named corpus file can never be parsed as a cpp option and cpp never
|
||||
opens the file's own directory for includes."""
|
||||
f = tmp_path / "-Ietc.F90" # a name that would be an option if passed as argv
|
||||
f.write_text("program x\nend program x\n")
|
||||
|
||||
captured = _capture_cpp_argv(monkeypatch)
|
||||
|
||||
out = extract._cpp_preprocess(f)
|
||||
assert out == b"preprocessed"
|
||||
last_arg = captured["argv"][-1]
|
||||
# Test the property, not the spelling: a leading "/" is only what "absolute"
|
||||
# looks like on POSIX, so the literal check failed on a perfectly correct
|
||||
# Windows path (C:\...\weird.F90). os.path.isabs answers for the host.
|
||||
assert os.path.isabs(last_arg), f"path arg must be absolute, got {last_arg!r}"
|
||||
assert not last_arg.startswith("-"), "path arg must never look like an option"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("hostile_name", ["-Ietc.F90", "-include.F90"])
|
||||
def test_cpp_preprocess_absolutises_a_relative_attacker_named_file(
|
||||
tmp_path, monkeypatch, hostile_name
|
||||
):
|
||||
"""The guard only does work when the incoming path is RELATIVE.
|
||||
|
||||
The test above hands in an already-absolute path, so it passes whether or
|
||||
not `_cpp_preprocess` resolves anything — removing the `.resolve()` does not
|
||||
make it fail. This is the case the hardening actually exists for: a corpus
|
||||
file whose own name is a cpp option, reached by a relative path.
|
||||
"""
|
||||
(tmp_path / hostile_name).write_text("program x\nend program x\n")
|
||||
monkeypatch.chdir(tmp_path)
|
||||
|
||||
captured = _capture_cpp_argv(monkeypatch)
|
||||
captured = _capture_cpp_call(monkeypatch)
|
||||
out = extract._cpp_preprocess(Path("-Ietc.F90"))
|
||||
|
||||
assert extract._cpp_preprocess(Path(hostile_name)) == b"preprocessed"
|
||||
last_arg = captured["argv"][-1]
|
||||
assert os.path.isabs(last_arg), (
|
||||
f"relative path was passed through unresolved: {last_arg!r}"
|
||||
)
|
||||
assert not last_arg.startswith("-"), (
|
||||
f"cpp would parse {last_arg!r} as an option, not a filename"
|
||||
assert out == b"preprocessed"
|
||||
argv = captured["argv"]
|
||||
assert argv == ["cpp", "-w", "-P", "-nostdinc", "-I", "/dev/null"]
|
||||
# No source path in argv at all — it goes through stdin.
|
||||
assert captured["input"] is not None
|
||||
assert b"program x" in captured["input"]
|
||||
|
||||
|
||||
def _read_secret_from(source_file: Path) -> bool:
|
||||
"""True if _cpp_preprocess leaked the sentinel secret into its output."""
|
||||
out = extract._cpp_preprocess(source_file)
|
||||
return b"TOP_SECRET_SENTINEL" in out
|
||||
|
||||
|
||||
@pytest.mark.skipif(shutil.which("cpp") is None, reason="cpp not available")
|
||||
def test_absolute_include_cannot_read_host_file(tmp_path):
|
||||
secret = tmp_path / "secret.txt"
|
||||
secret.write_text("TOP_SECRET_SENTINEL=abc123\n")
|
||||
src = tmp_path / "corpus" / "evil_abs.F90"
|
||||
src.parent.mkdir(parents=True)
|
||||
src.write_text(f'program evil\n#include "{secret}"\nend program evil\n')
|
||||
|
||||
assert not _read_secret_from(src), "absolute #include leaked a host file"
|
||||
|
||||
|
||||
@pytest.mark.skipif(shutil.which("cpp") is None, reason="cpp not available")
|
||||
def test_relative_traversal_include_cannot_escape_the_corpus(tmp_path):
|
||||
secret = tmp_path / "secret.txt"
|
||||
secret.write_text("TOP_SECRET_SENTINEL=abc123\n")
|
||||
src = tmp_path / "corpus" / "evil_rel.F90"
|
||||
src.parent.mkdir(parents=True)
|
||||
# ../secret.txt escapes corpus/ up to tmp_path/secret.txt
|
||||
src.write_text('program evil\n#include "../secret.txt"\nend program evil\n')
|
||||
|
||||
assert not _read_secret_from(src), "traversing #include escaped the corpus"
|
||||
|
||||
|
||||
@pytest.mark.skipif(shutil.which("cpp") is None, reason="cpp not available")
|
||||
def test_same_directory_include_also_stripped(tmp_path):
|
||||
# Even a same-directory include is stripped (blunt but safe); its contents
|
||||
# must not appear in the output.
|
||||
(tmp_path / "sensitive.inc").write_text("TOP_SECRET_SENTINEL=inline\n")
|
||||
src = tmp_path / "evil_same.F90"
|
||||
src.write_text('program evil\n#include "sensitive.inc"\nend program evil\n')
|
||||
|
||||
assert not _read_secret_from(src)
|
||||
|
||||
|
||||
@pytest.mark.skipif(shutil.which("cpp") is None, reason="cpp not available")
|
||||
def test_macro_expansion_still_works(tmp_path):
|
||||
"""The reason capital-F files need cpp — #define/#ifdef — must still work
|
||||
after the include hardening."""
|
||||
src = tmp_path / "macros.F90"
|
||||
src.write_text(
|
||||
"#define GREET hello_world\n"
|
||||
"#ifdef GREET\n"
|
||||
"program GREET\n"
|
||||
"end program GREET\n"
|
||||
"#endif\n"
|
||||
)
|
||||
out = extract._cpp_preprocess(src)
|
||||
if b"#define GREET" in out:
|
||||
# Host cpp is not a functional GNU-style preprocessor over this input
|
||||
# (e.g. the macOS clang `cpp` wrapper); _cpp_preprocess fell back to the
|
||||
# sanitized raw bytes. Security still holds (covered above); macro
|
||||
# fidelity is only available where cpp actually runs (Linux/CI).
|
||||
pytest.skip("host cpp did not preprocess the input")
|
||||
assert b"hello_world" in out
|
||||
assert b"GREET" not in out # the macro was expanded, not left verbatim
|
||||
Reference in new issue
Block a user