Skip to content

Commit 1161cd3

Browse files
committed
Keep eval visible after nested quoted command substitution
1 parent eaefff7 commit 1161cd3

2 files changed

Lines changed: 27 additions & 10 deletions

File tree

‎src/shell/run-shell-authz.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,13 @@ describe("quoted arguments are not command-position eval", () => {
403403
expect(runShellAuthzBlockReason(`echo "$(eval echo pwned)"`)).toMatch(destructive);
404404
});
405405

406+
test("nested quoted substitution does not hide sibling eval", () => {
407+
expect(runShellAuthzBlockReason(`echo "$(foo "$(true)" ; eval echo pwned)"`)).toMatch(destructive);
408+
expect(runShellAuthzBlockReason(`git commit -m "$(foo "$(true)" ; eval echo pwned)"`)).toMatch(destructive);
409+
expect(runShellAuthzBlockReason(`bash -c "$(echo "$(true)"; eval echo pwned)"`)).toMatch(destructive);
410+
expect(runShellAuthzBlockReason(`echo "$(echo "$(true)" && eval echo pwned)"`)).toMatch(destructive);
411+
});
412+
406413
test("eval after a closed quoted -m is still denied", () => {
407414
expect(runShellAuthzBlockReason(`git commit -m "fix" ; eval echo pwned`)).toMatch(destructive);
408415
});

‎src/shell/run-shell-authz.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -878,11 +878,22 @@ function isCatastrophicRm(segment: string): boolean {
878878
}
879879

880880
// Blank quoted interiors so CMD does not treat `;` inside `-m` text as a new command.
881+
// Double-quoted `$(...)` / backticks stay visible: their contents are real commands.
881882
function skipQuotedSpans(command: string): string {
882883
let out = "";
883884
let quote: '"' | "'" | undefined;
884-
let substDepth = 0;
885+
// Quote to restore when each `$(...)` closes. Extra `(` inside a substitution
886+
// is a `"paren"` frame so its `)` does not restore quote. A depth counter that
887+
// only restores at 0 treats the `"` after inner `"$(...)"` as an opener and
888+
// blanks sibling eval.
889+
const substStack: Array<'"' | "'" | undefined | "paren"> = [];
885890
let inBacktick = false;
891+
892+
const enterSubst = (): void => {
893+
substStack.push(quote);
894+
quote = undefined;
895+
};
896+
886897
for (let i = 0; i < command.length; i++) {
887898
const ch = command[i]!;
888899
if (quote === "'") {
@@ -910,8 +921,7 @@ function skipQuotedSpans(command: string): string {
910921
continue;
911922
}
912923
if (ch === "$" && command[i + 1] === "(") {
913-
substDepth++;
914-
quote = undefined;
924+
enterSubst();
915925
out += "$(";
916926
i++;
917927
continue;
@@ -930,21 +940,21 @@ function skipQuotedSpans(command: string): string {
930940
out += ch;
931941
continue;
932942
}
933-
if (substDepth > 0 && ch === "$" && command[i + 1] === "(") {
934-
substDepth++;
943+
if (substStack.length > 0 && ch === "$" && command[i + 1] === "(") {
944+
enterSubst();
935945
out += "$(";
936946
i++;
937947
continue;
938948
}
939-
if (substDepth > 0 && ch === "(") {
940-
substDepth++;
949+
if (substStack.length > 0 && ch === "(") {
950+
substStack.push("paren");
941951
out += ch;
942952
continue;
943953
}
944-
if (substDepth > 0 && ch === ")") {
945-
substDepth--;
954+
if (substStack.length > 0 && ch === ")") {
955+
const frame = substStack.pop();
946956
out += ch;
947-
if (substDepth === 0) quote = '"';
957+
if (frame !== "paren") quote = frame;
948958
continue;
949959
}
950960
if (ch === '"' || ch === "'") quote = ch;

0 commit comments

Comments
 (0)