From 087a7290264cc6fb7154ea8c2552a7b2cb8b33a3 Mon Sep 17 00:00:00 2001 From: orta Date: Tue, 16 Jun 2026 17:53:47 +0100 Subject: [PATCH] Adds a fix for a potential local shell execution based on a filename - thanks to @dremig --- CHANGELOG.md | 19 ++++++++------ .../git/_tests/localGetFileAtSHA.test.ts | 25 +++++++++++++++++++ source/platforms/git/localGetFileAtSHA.ts | 10 +++++--- 3 files changed, 43 insertions(+), 11 deletions(-) create mode 100644 source/platforms/git/_tests/localGetFileAtSHA.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ba73243d3..93f6c9d54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ +## 13.0.6 + +- Fixes a `danger local` shell execution possibility [@Junming] + ## 13.0.5 - Dropped p-limit as a dep @@ -2032,10 +2036,6 @@ That should do ya. I think. This doesn't support babel, and I haven't explored u Not usable for others, only stubs of classes etc. - [@orta] [@417-72ki]: https://github.com/417-72KI -[@HonzaMac]: https://github.com/HonzaMac -[@JanStevens]: https://github.com/JanStevens -[@RDIL]: https://github.com/RDIL -[@Teamop]: https://github.com/Teamop [@adam-bratin]: https://github.com/adam-bratin [@adam-moss]: https://github.com/adam-moss [@adamnoakes]: https://github.com/adamnoakes @@ -2059,10 +2059,11 @@ Not usable for others, only stubs of classes etc. - [@orta] [@dbgrandi]: https://github.com/dbgrandi [@dblandin]: https://github.com/dblandin [@denieler]: https://github.com/denieler -[@dimitar-hristov]: https://github.com/dimitar-hristov [@dfalling]: https://github.com/dfalling +[@dimitar-hristov]: https://github.com/dimitar-hristov [@dkundel]: https://github.com/dkundel [@doniyor2109]: https://github.com/doniyor2109 +[@dremig]: https://github.com/dremig [@ds300]: https://github.com/ds300 [@f-meloni]: https://github.com/f-meloni [@fbartho]: https://github.com/fbartho @@ -2082,12 +2083,14 @@ Not usable for others, only stubs of classes etc. - [@orta] [@hmcc]: https://github.com/hmcc [@hmschreiner]: https://github.com/hmschreiner [@hongrich]: https://github.com/hongrich +[@HonzaMac]: https://github.com/HonzaMac [@igorbek]: https://github.com/igorbek [@iljadaderko]: https://github.com/IljaDaderko [@imorente]: https://github.com/imorente [@ivankatliarchuk]: https://github.com/ivankatliarchuk [@jamiebuilds]: https://github.com/jamiebuilds [@jamime]: https://github.com/jamime +[@JanStevens]: https://github.com/JanStevens [@joarwilk]: https://github.com/joarwilk [@johansteffner]: https://github.com/johansteffner [@jonny133]: https://github.com/jonny133 @@ -2125,11 +2128,12 @@ Not usable for others, only stubs of classes etc. - [@orta] [@pgoudreau]: https://github.com/@pgoudreau [@pinkasey]: https://github.com/pinkasey [@pveyes]: https://github.com/pveyes +[@radimsv]: https://github.com/radimsv [@randak]: https://github.com/randak [@ravanscafi]: https://github.com/ravanscafi +[@RDIL]: https://github.com/RDIL [@rogerluan]: https://github.com/rogerluan [@rohit-gohri]: https://github.com/rohit-gohri -[@radimsv]: https://github.com/radimsv [@rouby]: https://github.com/rouby [@rzgry]: https://github.com/rzgry [@sajjadzamani]: https://github.com/sajjadzamani @@ -2146,10 +2150,11 @@ Not usable for others, only stubs of classes etc. - [@orta] [@stevemoser]: https://github.com/stevemoser [@stevenp]: https://github.com/stevenp [@sunshinejr]: https://github.com/sunshinejr +[@Teamop]: https://github.com/Teamop +[@thawankeane]: https://github.com/thawankeane [@thii]: https://github.com/thii [@tibdex]: https://github.com/tibdex [@tim3trick]: https://github.com/tim3trick -[@thawankeane]: https://github.com/thawankeane [@tomstrepsil: https://github.com/TomStrepsil] [@tychota]: https://github.com/tychota [@urkle]: https://github.com/urkle diff --git a/source/platforms/git/_tests/localGetFileAtSHA.test.ts b/source/platforms/git/_tests/localGetFileAtSHA.test.ts new file mode 100644 index 000000000..8fa085ae5 --- /dev/null +++ b/source/platforms/git/_tests/localGetFileAtSHA.test.ts @@ -0,0 +1,25 @@ +import { execFile } from "child_process" + +import { localGetFileAtSHA } from "../localGetFileAtSHA" + +jest.mock("child_process", () => ({ + __esModule: true, + execFile: jest.fn((_cmd, _args, callback) => callback(null, "", "")), +})) + +it("invokes git via execFile with an argv array (no shell)", async () => { + await localGetFileAtSHA("src/index.ts", undefined, "abc123") + + expect(execFile).toHaveBeenCalledWith("git", ["show", "abc123:src/index.ts"], expect.any(Function)) +}) + +it("passes a malicious file path as a single literal argument so it cannot inject commands", async () => { + // A path crafted to break out of a quoted shell argument and run `touch` + const evilPath = `x";touch /tmp/pwn;#/file.txt` + + await localGetFileAtSHA(evilPath, undefined, "master") + + // The whole `sha:path` must arrive as one argv entry — never split into a + // shell command — so the metacharacters are inert. + expect(execFile).toHaveBeenCalledWith("git", ["show", `master:${evilPath}`], expect.any(Function)) +}) diff --git a/source/platforms/git/localGetFileAtSHA.ts b/source/platforms/git/localGetFileAtSHA.ts index 4416f6cd9..67f4a7042 100644 --- a/source/platforms/git/localGetFileAtSHA.ts +++ b/source/platforms/git/localGetFileAtSHA.ts @@ -1,14 +1,16 @@ import { debug } from "../../debug" -import { exec } from "child_process" +import { execFile } from "child_process" const d = debug("localGetFileAtSHA") export const localGetFileAtSHA = (path: string, _repo: string | undefined, sha: string) => new Promise((done) => { - const call = `git show ${sha}:"${path}"` - d(call) + // Use execFile with an argv array (no shell) so that file paths containing + // shell metacharacters or quotes cannot break out and inject commands. + const args = ["show", `${sha}:${path}`] + d(`git ${args.join(" ")}`) - exec(call, (err, stdout, _stderr) => { + execFile("git", args, (err, stdout, _stderr) => { if (err) { console.error(`Could not get the file ${path} from git at ${sha}`) console.error(err)