From 506ffa05bf6e14cab82208d3befc7aa44dd980f5 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 26 May 2026 13:10:34 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]=20Fi?= =?UTF-8?q?x=20terminal=20injection=20via=20ST=20sequence=20bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ANSI stripping logic previously only recognized `\u0007` (Bell) as a terminator for OSC (Operating System Command) sequences. Malicious output could bypass sanitization by using the standard String Terminator (ST) `\u001b\`. This commit updates the ANSI regex to match both terminators, and fixes the streaming state logic to correctly handle partial chunks when terminators are split or back-to-back. Tests have been added to verify ST sequence handling. Co-authored-by: hongymagic <302730+hongymagic@users.noreply.github.com> --- .jules/sentinel.md | 5 +++++ src/ansi.ts | 25 +++++++++++++++++++++++-- tests/ansi.test.ts | 10 ++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index e644cad..04a3df0 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -39,3 +39,8 @@ **Vulnerability:** The `filterSensitiveFields` function in `src/providers/index.ts` used a manual recursive loop to drop sensitive keys, but it failed to recurse into arrays. If a configuration object contained an array with sensitive nested objects, those secrets would be leaked in debug logs. **Learning:** Manual object traversal for redaction is prone to edge cases (like arrays or circular references). **Prevention:** Use a custom replacer function with `JSON.stringify` to safely and completely redact sensitive fields across all nested structures. + +## 2026-05-26 - Terminal Injection via ST (String Terminator) Sequences +**Vulnerability:** The ANSI stripping logic only supported `\u0007` (Bell) as the terminator for Operating System Command (OSC) sequences. It failed to support the alternative ANSI standard terminator `\u001b\` (String Terminator, or ST). An attacker could bypass output sanitization by terminating a malicious hyperlink or other OSC sequence with `\u001b\`, leading to terminal injection. +**Learning:** ANSI standards are complex and support multiple escape sequences. Relying on partial implementations (like only supporting BEL) leaves gaps that can be exploited for terminal injection when the output contains untrusted AI generation. Stateful stream chunking makes partial matching especially difficult if the terminator spans chunk boundaries. +**Prevention:** Update ANSI removal regular expressions to fully match standard OSC terminators, specifically `(?:\u0007|\u001B\\)`. Ensure stream-chunk buffering logic correctly identifies the starts and ends of sequences, even when the terminator itself is a multi-character escape sequence. diff --git a/src/ansi.ts b/src/ansi.ts index dd2c205..5c397b4 100644 --- a/src/ansi.ts +++ b/src/ansi.ts @@ -2,7 +2,7 @@ // eslint-disable-next-line no-control-regex const ANSI_REGEX = new RegExp( [ - "[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)", + "[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?(?:\\u0007|\\u001B\\\\))", "(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]))", ].join("|"), "g", @@ -59,11 +59,32 @@ export function createAnsiStripper() { // If the buffer ends with a partial ANSI sequence (starts with ESC), keep it in buffer // ESC is \u001B or \u009B - const lastEscIndex = Math.max( + let lastEscIndex = Math.max( buffer.lastIndexOf("\u001B"), buffer.lastIndexOf("\u009B"), ); + // If the last ESC is part of an ST terminator (\u001B\\), it's not the start + // of an incomplete sequence, it's the end of an OSC sequence. We should look + // for the ESC that precedes it to find the actual start of a potentially + // incomplete sequence. + if ( + lastEscIndex !== -1 && + buffer.slice(lastEscIndex, lastEscIndex + 2) === "\u001B\\" + ) { + let checkIndex = lastEscIndex; + while ( + checkIndex !== -1 && + buffer.slice(checkIndex, checkIndex + 2) === "\u001B\\" + ) { + checkIndex = Math.max( + buffer.lastIndexOf("\u001B", checkIndex - 1), + buffer.lastIndexOf("\u009B", checkIndex - 1), + ); + } + lastEscIndex = checkIndex; + } + if (lastEscIndex !== -1) { // Check if this looks like an incomplete sequence // A complete sequence usually ends with a letter or specific symbols diff --git a/tests/ansi.test.ts b/tests/ansi.test.ts index 0153c5f..6acf4c5 100644 --- a/tests/ansi.test.ts +++ b/tests/ansi.test.ts @@ -35,3 +35,13 @@ test("createAnsiStripper handles alternating plain text and ANSI chunks", () => expect(stripper("\u001b[33myellow\u001b[0m")).toBe("yellow"); expect(stripper("final")).toBe("final"); }); + +test("createAnsiStripper handles ST sequences", () => { + const stripper = createAnsiStripper(); + // String terminated with ST sequence (\u001b\) + const stSequence = + "\u001b]8;;http://example.com\u001b\\Link\u001b]8;;\u001b\\"; + + // Depending on whether it splits or is in one chunk, the result should be just the link text + expect(stripper(stSequence)).toBe("Link"); +});