mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(security): polynomial ReDoS in comboAgentMiddleware regex → main (#3983)
Brings the release/v3.8.27 fix (#3982) to main so CodeQL alerts #612/#613 close on the next scan. Code + regression test only; the [3.8.27] CHANGELOG bullet lives on release/v3.8.27 and reaches main when v3.8.27 ships (identical file → no merge conflict). Detection pattern drops the unbounded surrounding newline run; global strip pattern bounds it ({0,16}). Behavior unchanged (107 related tests green).
This commit is contained in:
committed by
GitHub
parent
ca1e17f740
commit
7509a32e9d
@@ -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 <omniModel> 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>([^<]+)<\/omniModel>(?:\\n|\n|\r)*/;
|
||||
const CACHE_TAG_PATTERN = /<omniModel>([^<]+)<\/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
|
||||
// <omniModel> 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>([^<]+)<\/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
|
||||
// <omniModel> 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>([^<]+)<\/omniModel>(?:\\n|\n|\r){0,16}/g;
|
||||
|
||||
/**
|
||||
* Inject the model tag into the last assistant message (or append a new one).
|
||||
|
||||
@@ -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 — <omniModel> 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) + "<omniModel" + "x".repeat(2000);
|
||||
const start = process.hrtime.bigint();
|
||||
stripModelTags([{ role: "user", content: evil }]);
|
||||
const elapsedMs = Number(process.hrtime.bigint() - start) / 1e6;
|
||||
assert.ok(elapsedMs < 1000, `regex must stay linear; took ${elapsedMs.toFixed(1)}ms`);
|
||||
});
|
||||
|
||||
test("extractPinnedModel still finds a tag wrapped in newlines", () => {
|
||||
const messages = [
|
||||
{
|
||||
role: "assistant",
|
||||
content: "answer\n\n<omniModel>claude/claude-opus-4-8</omniModel>\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\n<omniModel>a/b</omniModel>\n\nafter" }])[0]
|
||||
.content
|
||||
);
|
||||
assert.ok(!out.includes("<omniModel>"), "tag removed");
|
||||
assert.ok(!out.includes("\n\n\n"), "no triple newline left from stripping");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user