From d173a651a09179b0006e1933466f1dd39c8f0e8b Mon Sep 17 00:00:00 2001 From: gus Date: Thu, 7 May 2026 23:15:50 -0300 Subject: [PATCH] security: pass cwd to git via execFileSync, not interpolation through /bin/sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bin/gstack-memory-ingest.ts:632-643` ran `execSync(\`git -C ${JSON.stringify(cwd)} remote get-url origin 2>/dev/null\`, ...)`. JSON.stringify escapes `"` and `\` but not `$` or backticks, so a `cwd` of `"$(touch /tmp/marker)"` survived JSON quoting and detonated under /bin/sh's command-substitution-inside-double-quotes. `cwd` originates from transcript JSONL records under `~/.claude/projects//.jsonl` and `~/.codex/sessions/YYYY/MM/DD/rollout-*.jsonl`. The walker grabs the first `.cwd` it sees per session. That's an untrusted surface in the gstack threat model — the L1-L6 sidebar security stack exists exactly because agent transcripts can carry attacker-influenced text. Two pivots above the local same-uid bar: (a) prompt-injection appending `cwd="$(...)"` to the active session log turns the next /sync-gbrain run into RCE under the user's uid; (b) cross-machine transcript share (a colleague's `.claude/projects` snippet untar'd into HOME, a documented gbrain dogfooding shape) → RCE on first sync. Fix swaps the one execSync for `execFileSync("git", ["-C", cwd, "remote", "get-url", "origin"], ...)`. No shell, argv passed directly to git. The same module already uses execFileSync for `gbrainAvailable()` (line 762 pre-patch) and `gbrainPutPage()` (line 816 pre-patch) — this single execSync was the outlier. Test: `gstack-memory-ingest security: untrusted cwd cannot trigger shell substitution` plants a Claude-Code-shaped JSONL with cwd=`$(touch )` and asserts the marker file is not created after `--incremental --quiet`. Negative control: with the patch reverted, the test fails (marker created); with the patch applied, it passes (18/18 in test/gstack-memory-ingest.test.ts). --- bin/gstack-memory-ingest.ts | 9 +++++- test/gstack-memory-ingest.test.ts | 46 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index 5d3401e03..56b072b2b 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -632,9 +632,16 @@ function extractContentText(rec: any): string { function resolveGitRemote(cwd: string): string { if (!cwd) return ""; try { - const out = execSync(`git -C ${JSON.stringify(cwd)} remote get-url origin 2>/dev/null`, { + // execFileSync (no shell) so `cwd` cannot trigger command substitution. + // Transcript JSONL records are an untrusted surface (a poisoned `.cwd` + // value containing `"$(...)"` survived `JSON.stringify` interpolation + // into a `/bin/sh -c` context, since JSON quoting does not escape `$` + // or backticks). Mirrors the execFileSync pattern this module already + // uses for `gbrainAvailable()` (line 762) and `gbrainPutPage()` (line 816). + const out = execFileSync("git", ["-C", cwd, "remote", "get-url", "origin"], { encoding: "utf-8", timeout: 2000, + stdio: ["ignore", "pipe", "ignore"], }); return canonicalizeRemote(out.trim()); } catch { diff --git a/test/gstack-memory-ingest.test.ts b/test/gstack-memory-ingest.test.ts index 5fb6ebbfa..fec698271 100644 --- a/test/gstack-memory-ingest.test.ts +++ b/test/gstack-memory-ingest.test.ts @@ -205,6 +205,52 @@ describe("gstack-memory-ingest state file", () => { }); }); +// ── Security: cwd in transcript JSONL must not reach a shell ───────────── + +describe("gstack-memory-ingest security: untrusted cwd cannot trigger shell substitution", () => { + it("does not invoke /bin/sh when a transcript record contains $() in cwd", () => { + // Transcript JSONL is an untrusted surface — a record's `.cwd` value + // can be set by anyone who can write to ~/.claude/projects (cross-machine + // share, prompt-injection appending to the active session log, etc.). + // resolveGitRemote() must use execFileSync, not execSync with template + // interpolation, or `cwd="$(...)"` triggers command substitution under + // /bin/sh -c on the next ingest run. + const home = makeTestHome(); + const gstackHome = join(home, ".gstack"); + mkdirSync(gstackHome, { recursive: true }); + + const markerDir = mkdtempSync(join(tmpdir(), "gstack-mi-cwd-marker-")); + const marker = join(markerDir, "PWNED"); + // Plain $(...) — what an attacker would write into a transcript record. + // execFileSync passes this verbatim to git as a -C argument; execSync + // (the prior code path) wrapped it in a /bin/sh -c template that ran + // the substitution. + const malicious = "$(touch " + marker + ")"; + + const record = JSON.stringify({ + type: "user", + uuid: "11111111-1111-1111-1111-111111111111", + sessionId: "abc", + cwd: malicious, + timestamp: new Date().toISOString(), + message: { role: "user", content: "hi" }, + }); + writeClaudeCodeSession(home, "-tmp-target", "abc", record + "\n"); + + const r = runScript(["--incremental", "--quiet"], { + HOME: home, + GSTACK_HOME: gstackHome, + GSTACK_MEMORY_INGEST_NO_WRITE: "1", + }); + + expect(r.exitCode).toBe(0); + expect(existsSync(marker)).toBe(false); + + rmSync(home, { recursive: true, force: true }); + rmSync(markerDir, { recursive: true, force: true }); + }); +}); + // ── Transcript parser via re-import of the source module ─────────────────── describe("internal: parseTranscriptJsonl + buildTranscriptPage shape", () => {