Skip to content

Commit 3c2840e

Browse files
committed
Close backslash-escaped-quote bypass; narrow changelog claim
dequoteForMatching tracked quote state without escape awareness, so a backslash-escaped quote (\") still toggled quote state the same as a real one. In real bash \" is a literal quote character that never opens or closes a quoted span, so an operator or flag that follows is genuinely unquoted. Skip the escaped character without touching quote state. Also narrow the CHANGELOG's nested bash -c claim to what the tests actually cover (one level of quoting inside -c), not true multi-level nested-shell parsing, which remains a known gap tracked separately.
1 parent 0d44afc commit 3c2840e

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

‎src/permission/auto-shell-policy.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,24 @@ const inCmd = (body: string): RegExp => new RegExp(`${CMD}${body}`);
4545
// span; every other character (letters, digits, `-`) passes through
4646
// dequoted. Heredoc bodies are left alone: the file-mutation heredoc pattern
4747
// keys on the bare `<<` operator, which is always outside any quoting.
48+
//
49+
// A backslash before a quote character escapes it: `\"` is a literal `"`
50+
// that never opens or closes a quoted span (real bash semantics outside
51+
// single quotes), so `echo hi \"> file"` is a bare, unquoted redirect, not
52+
// text inside a quote. Skip the escaped character without touching quote
53+
// state so its following operator is still seen as live.
4854
const QUOTE_NEUTRALIZED_OPERATORS = new Set(["<", ">", "|", "&", ";", "`"]);
4955

5056
const dequoteForMatching = (command: string): string => {
5157
let out = "";
5258
let quote: '"' | "'" | null = null;
53-
for (const ch of command) {
59+
for (let i = 0; i < command.length; i++) {
60+
const ch = command[i] as string;
61+
if (ch === "\\" && quote !== "'" && i + 1 < command.length) {
62+
out += command[i + 1];
63+
i++;
64+
continue;
65+
}
5466
if (quote !== null) {
5567
if (ch === quote) {
5668
quote = null;

‎src/permission/classify-security.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,23 @@ describe("CL-6703 — quoted redirect targets still deny file-mutation", () => {
736736
test("a quoted '>' inside non-redirect text does not false-positive", () => {
737737
expect(autoShellRuleForCall(shellCall(`git commit -m 'fix > bug'`))).toBeUndefined();
738738
});
739+
740+
test("a backslash-escaped quote before a redirect still denies", () => {
741+
// `\"` is a literal quote character in real bash, not a quote-open — the
742+
// shell is never inside a quoted string here, so the `>` that follows is
743+
// a genuine, unquoted redirect.
744+
expect(autoShellRuleForCall(shellCall('echo hi \\"> file"'))?.name).toBe("file-mutation");
745+
});
746+
747+
test("a backslash-escaped quote ahead of a dangerous flag still denies", () => {
748+
// The escaped quote sits before an extra leading space, so it never
749+
// touches the `\s-c` junction later in the string; a naive quote-pairing
750+
// scanner (ignoring the backslash) would consume that junction as part
751+
// of a fake quoted span and hide the -c flag entirely.
752+
expect(autoShellRuleForCall(shellCall('python3 \\" -c print(1)"'))?.name).toBe(
753+
"file-mutation",
754+
);
755+
});
739756
});
740757

741758
describe("CL-6702 — bash clobber redirects match file-mutation", () => {

0 commit comments

Comments
 (0)