From a9d73bcbfba17797424b8ca034a2cb8abb99a26a Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 16 Jun 2026 08:42:58 -0300 Subject: [PATCH] fix(security): polynomial ReDoS in comboAgentMiddleware regex (#3982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): eliminate polynomial ReDoS in comboAgentMiddleware regex (CodeQL js/polynomial-redos) CACHE_TAG_PATTERN wrapped the tag in an unbounded `(?:\\n|\n|\r)*` prefix/suffix. On an unanchored `.test()`/`.exec()` that is O(nĀ²) on inputs with many newlines (CodeQL js/polynomial-redos, alerts #612/#613). The surrounding runs are irrelevant to detecting/capturing the tag, so the detection pattern now matches only the core `([^<]+)`; the global strip pattern still consumes the wrapping newlines (combo.ts streaming, #531) but BOUNDED ({0,16}) so it stays linear. Behavior preserved: detection, model extraction, multi-tag stripping (#454) and blank-line cleanup all unchanged (107 related tests green). Adds ReDoS-safety regression tests (50k-newline inputs complete in <1ms). * docs(changelog): add #3982 ReDoS fix to [3.8.27] --- CHANGELOG.md | 4 +- open-sse/services/comboAgentMiddleware.ts | 27 +++++++----- .../combo-omnimodel-tag-stripping.test.ts | 43 +++++++++++++++++++ 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c081a84c1a..bbf15f386a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,9 @@ ## [3.8.27] ā€” TBD -_Cycle open ā€” entries are added here as PRs merge into `release/v3.8.27`._ +### šŸ”’ Security & Hardening + +- **fix(security): eliminate a polynomial ReDoS in the combo `` tag regex** ā€” `comboAgentMiddleware`'s cache-tag pattern wrapped the tag in an unbounded newline run (`(?:\n|\r)*`), making `.test()` / `.replace()` run in O(nĀ²) on inputs with many newlines (CodeQL `js/polynomial-redos`). The detection pattern now matches only the core `ā€¦` and the global strip pattern bounds the surrounding newline runs, keeping it linear; detection / extraction / multi-tag stripping behavior is unchanged. ([#3982](https://github.com/diegosouzapw/OmniRoute/pull/3982) ā€” thanks @diegosouzapw) --- diff --git a/open-sse/services/comboAgentMiddleware.ts b/open-sse/services/comboAgentMiddleware.ts index 28b5de5f4f..f93de4d285 100644 --- a/open-sse/services/comboAgentMiddleware.ts +++ b/open-sse/services/comboAgentMiddleware.ts @@ -34,18 +34,25 @@ interface Message { // ā”€ā”€ Context Caching Tag ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ -// Handles both actual newlines (U+000A) and literal \n sequences injected -// by combo.ts streaming around the tag (#531). Non-global so that -// .exec() and .test() stay stateless (a global regex carries lastIndex between +// Detection / extraction pattern. The newline runs combo.ts streaming wraps the +// tag with (#531) are irrelevant to *finding* the tag or capturing the model id, +// so they are intentionally NOT matched here: an unbounded `(?:\\n|\n|\r)*` prefix +// on this unanchored regex made `.test()` / `.exec()` run in O(nĀ²) on inputs with +// many newlines (polynomial ReDoS ā€” CodeQL js/polynomial-redos, #3870). Non-global +// so `.exec()` / `.test()` stay stateless (a global regex carries lastIndex between // calls and would skip matches). -const CACHE_TAG_PATTERN = /(?:\\n|\n|\r)*([^<]+)<\/omniModel>(?:\\n|\n|\r)*/; +const CACHE_TAG_PATTERN = /([^<]+)<\/omniModel>/; -// Global variant for .replace() callers that must strip EVERY tag. A non-global -// regex only removes the first match, so a single message carrying more than one -// tag (e.g. an Open WebUI follow-up/title request that inlines the -// whole chat history) leaked the remaining tags to the provider ā€” defeating the -// cache-session protection stripModelTags exists to enforce (#454). -const CACHE_TAG_PATTERN_GLOBAL = /(?:\\n|\n|\r)*([^<]+)<\/omniModel>(?:\\n|\n|\r)*/g; +// Global variant for `.replace()` callers that must strip EVERY tag (a non-global +// regex only removes the first match, so a message carrying more than one +// tag ā€” e.g. an Open WebUI follow-up/title request that inlines the +// whole chat history ā€” leaked the remaining tags to the provider, defeating the +// cache-session protection stripModelTags enforces, #454). This variant still +// consumes the newline run wrapping the tag (combo.ts streaming, #531) so removal +// leaves no blank line, but the runs are BOUNDED ({0,16}) to keep the regex linear +// (no polynomial backtracking, #3870); 16 is far beyond any real streaming wrap. +const CACHE_TAG_PATTERN_GLOBAL = + /(?:\\n|\n|\r){0,16}([^<]+)<\/omniModel>(?:\\n|\n|\r){0,16}/g; /** * Inject the model tag into the last assistant message (or append a new one). diff --git a/tests/unit/combo-omnimodel-tag-stripping.test.ts b/tests/unit/combo-omnimodel-tag-stripping.test.ts index fc6d5fa3c0..fa30559fdb 100644 --- a/tests/unit/combo-omnimodel-tag-stripping.test.ts +++ b/tests/unit/combo-omnimodel-tag-stripping.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { stripModelTags, injectModelTag, + extractPinnedModel, } from "../../open-sse/services/comboAgentMiddleware.ts"; // Regression for the non-global CACHE_TAG_PATTERN bug: `.replace()` with a @@ -56,3 +57,45 @@ describe("comboAgentMiddleware ā€” tag stripping (non-global regex b ); }); }); + +// Regression for CodeQL js/polynomial-redos (#3870): the unbounded `(?:\\n|\n|\r)*` +// prefix/suffix on the unanchored CACHE_TAG_PATTERN made `.test()` / `.replace()` +// run in O(nĀ²) on inputs with many newlines. The fix drops the surrounding runs from +// the detection pattern and bounds them ({0,16}) in the global strip pattern. +describe("comboAgentMiddleware ā€” ReDoS safety + newline-wrapped tags (#3870)", () => { + test("stripModelTags stays linear on a long run of newlines (no tag present)", () => { + const evil = "\n".repeat(50_000); + const start = process.hrtime.bigint(); + const stripped = stripModelTags([{ role: "user", content: evil }]); + const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6; + assert.equal(stripped[0].content, evil, "no tag ā†’ content returned unchanged"); + assert.ok(elapsedMs < 1000, `regex must stay linear; took ${elapsedMs.toFixed(1)}ms`); + }); + + test("stripModelTags stays linear on many newlines before a never-closing tag", () => { + const evil = "\n".repeat(50_000) + " { + const messages = [ + { + role: "assistant", + content: "answer\n\nclaude/claude-opus-4-8\n\n", + }, + ]; + assert.equal(extractPinnedModel(messages), "claude/claude-opus-4-8"); + }); + + test("stripModelTags removes the newline run wrapping a tag (no blank line left)", () => { + const out = String( + stripModelTags([{ role: "user", content: "before\n\na/b\n\nafter" }])[0] + .content + ); + assert.ok(!out.includes(""), "tag removed"); + assert.ok(!out.includes("\n\n\n"), "no triple newline left from stripping"); + }); +});