From 21c868e4b06d1c3a33a6d5f7147751f378bd45c3 Mon Sep 17 00:00:00 2001 From: sec-check Date: Mon, 5 Oct 2026 12:10:38 -0400 Subject: [PATCH 1/2] fix(security): decode CSS escapes before the remote-reference gate A browser's CSS tokenizer resolves escape sequences before a url() or @import value is ever read as a URL, so url(\68ttps://evil.example/x.css) fetches https://evil.example/x.css. findRemoteReferences() scanned the raw text, saw a value starting with a backslash -- neither absolute nor protocol-relative -- and let the fetch past the gate that keeps a visitor's IP, User-Agent and Referer off a host the diagram's author chose. Decode escapes in the CSS callers only: attribute values that are URLs rather than CSS keep a backslash's URL meaning, which remoteTarget() already handles. CSS_URL_PATTERN's bare branch needed widening too. The single whitespace character that terminates a hex escape belongs to the escape, so url(\000068 ttps://evil.example/x.css) is one token worth the full URL while [^)\s"']* captured only \000068 and lost the host. NUL, the surrogate range and out-of-range code points decode to U+FFFD as CSS specifies, so the decoder cannot invent a host no browser fetches. Signed-off-by: sec-check --- scripts/lib/svg-active-content.mjs | 63 ++++++++++++++++++- tests/svg-active-content.test.mjs | 97 ++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 3 deletions(-) diff --git a/scripts/lib/svg-active-content.mjs b/scripts/lib/svg-active-content.mjs index e61babeb..96342b07 100644 --- a/scripts/lib/svg-active-content.mjs +++ b/scripts/lib/svg-active-content.mjs @@ -278,8 +278,65 @@ function styleBlockContents(source) { return blocks; } -/** A CSS `url(...)` target, quoted or bare. Covers `@font-face` `src` too. */ -const CSS_URL_PATTERN = /url\(\s*(?:"([^"]*)"|'([^']*)'|([^)\s"']*))\s*\)/gi; +/** + * A CSS escape sequence: a hex escape of one to six digits with the single + * optional whitespace character that terminates it, a backslash-newline line + * continuation, or a backslash before any other single character. + */ +const CSS_ESCAPE_PATTERN = + /\\(?:([0-9a-f]{1,6})(?:\r\n|[ \n\r\t\f])?|(\r\n|[\n\r\f])|([\s\S]))/gi; + +/** + * Decode the CSS escape sequences in a url token or string. + * + * A browser's CSS tokenizer resolves escapes before the value is ever read as + * a URL, so `url(\68ttps://evil.example/x.css)` fetches + * `https://evil.example/x.css`. Scanning the raw text instead sees a value + * starting with a backslash, which is neither absolute nor protocol-relative, + * and the remote fetch walks past findRemoteReferences -- the gate that keeps + * a visitor's IP, User-Agent and Referer from reaching a host the diagram's + * author chose. Every escapable character is reachable this way, so no + * substring test on the undecoded value can stand in for decoding. + * + * This is CSS syntax only. Attribute values that are URLs rather than CSS keep + * a backslash's URL meaning, which remoteTarget already handles, so decoding + * stays scoped to the CSS callers. + * + * @param {string} value - Raw CSS url token or string contents. + * @returns {string} + */ +function decodeCssEscapes(value) { + return String(value).replace( + CSS_ESCAPE_PATTERN, + (match, hex, newline, literal) => { + if (hex !== undefined) { + const code = Number.parseInt(hex, 16); + // CSS maps NUL, the surrogate range and out-of-range code points to + // U+FFFD rather than to the character the digits name. + return code === 0 || + code > 0x10ffff || + (code >= 0xd800 && code <= 0xdfff) + ? '\ufffd' + : String.fromCodePoint(code); + } + // A backslash-newline inside a string is a line continuation: it + // contributes nothing, so the text either side joins up. + return newline !== undefined ? '' : literal; + }, + ); +} + +/** + * A CSS `url(...)` target, quoted or bare. Covers `@font-face` `src` too. + * + * The bare branch spells out escape sequences rather than stopping at the + * first whitespace, because the whitespace that terminates a hex escape + * belongs to the escape: `url(\000068 ttps://evil.example/x.css)` is one url + * token whose value is `https://evil.example/x.css`, and a `[^)\s"']*` read + * would capture only `\000068` and lose the host entirely. + */ +const CSS_URL_PATTERN = + /url\(\s*(?:"([^"]*)"|'([^']*)'|((?:\\[0-9a-f]{1,6}[ \n\r\t\f]?|\\[\s\S]|[^)\s"'\\])*))\s*\)/gi; /** * An `@import` whose target is a bare string rather than a `url(...)`. @@ -407,7 +464,7 @@ function cssTargets(css) { for (const pattern of [CSS_URL_PATTERN, CSS_IMPORT_PATTERN]) { for (const match of String(css).matchAll(pattern)) { const value = match[1] ?? match[2] ?? match[3]; - const target = remoteTarget(value); + const target = remoteTarget(decodeCssEscapes(value)); if (target) targets.push(target); } } diff --git a/tests/svg-active-content.test.mjs b/tests/svg-active-content.test.mjs index e83e4876..5ba78e87 100644 --- a/tests/svg-active-content.test.mjs +++ b/tests/svg-active-content.test.mjs @@ -1008,3 +1008,100 @@ test('accepts the UTF-8 encoding spellings a generator actually emits', () => { ); } }); + +test('a CSS hex escape in a bare url() does not hide the host', () => { + // A browser's CSS tokenizer resolves escapes before the value is read as a + // URL, so `url(\68ttps://...)` fetches `https://...`. Scanning the raw text + // instead sees a value starting with a backslash -- neither absolute nor + // protocol-relative -- and the remote fetch walked past the gate. + const svg = + ''; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), [ + 'references a remote resource in a '; + assert.deepEqual(findRemoteReferences(svg), []); +}); + +test('an interior backslash in a local CSS path stays local', () => { + const svg = + ''; + assert.deepEqual(findRemoteReferences(svg), []); +}); From 053d3c7f49bcd063824ee7f8e6ab2a12ad05a944 Mon Sep 17 00:00:00 2001 From: sec-check Date: Tue, 6 Oct 2026 00:15:57 -0400 Subject: [PATCH 2/2] fix(security): make CSS_URL_PATTERN's bare-url branch backtrack linearly The bare-url alternatives added in the previous commit overlapped on hex escapes: `\\[0-9a-f]{1,6}` could match `\abcde` and leave the trailing `f` to `[^)\s"'\\]`, and `\\[\s\S]` could take any of the same escapes a third way. With no closing `)` to stop the search, every repetition multiplied the number of ways to carve the same text, so an unterminated `url(` followed by repeated `\abcdef` grew about 7x per repetition: 8 reps 244ms, 10 reps 6.8s, and 400 reps does not finish. findRemoteReferences() runs over third-party SVGs in import-architectures, validate-architecture-assets and the enduser collector, so a single upstream diagram could hang each of those jobs -- the scanner meant to gate untrusted input was itself a denial-of-service sink for it. Make the alternatives mutually exclusive so each input has exactly one parse: a full six-digit escape, a shorter escape guarded by `(?![0-9a-f])` so it cannot split a longer run, a single-character escape that excludes hex digits, and the unchanged bare character class. Differential fuzzing over 300,000 random url() inputs found no match or capture-group difference against the old pattern, so this is a backtracking fix only and the escape-decoding behaviour is unchanged. The regression test scans 400 unterminated escapes and asserts it finishes within a second; against the old pattern it does not complete in 180s. Signed-off-by: sec-check --- scripts/lib/svg-active-content.mjs | 12 +++++++++++- tests/svg-active-content.test.mjs | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/scripts/lib/svg-active-content.mjs b/scripts/lib/svg-active-content.mjs index 96342b07..e618b710 100644 --- a/scripts/lib/svg-active-content.mjs +++ b/scripts/lib/svg-active-content.mjs @@ -334,9 +334,19 @@ function decodeCssEscapes(value) { * belongs to the escape: `url(\000068 ttps://evil.example/x.css)` is one url * token whose value is `https://evil.example/x.css`, and a `[^)\s"']*` read * would capture only `\000068` and lose the host entirely. + * + * The alternatives inside the bare branch are mutually exclusive on purpose. + * A single `\\[0-9a-f]{1,6}` alternative lets a hex run split across branches + * — `\abcdef` as `\abcde` plus a bare `f`, and so on — so an unterminated + * `url(` followed by repeated escapes backtracks exponentially (about 7x per + * repetition). findRemoteReferences runs on third-party SVGs, so that is a + * hang an upstream diagram can trigger. Splitting the hex run into a full + * six-digit form and a shorter form guarded by `(?![0-9a-f])`, and excluding + * hex digits from the single-character escape, leaves exactly one way to match + * any input and keeps the scan linear. */ const CSS_URL_PATTERN = - /url\(\s*(?:"([^"]*)"|'([^']*)'|((?:\\[0-9a-f]{1,6}[ \n\r\t\f]?|\\[\s\S]|[^)\s"'\\])*))\s*\)/gi; + /url\(\s*(?:"([^"]*)"|'([^']*)'|((?:\\[0-9a-f]{6}[ \n\r\t\f]?|\\[0-9a-f]{1,5}(?![0-9a-f])[ \n\r\t\f]?|\\[^0-9a-f]|[^)\s"'\\])*))\s*\)/gi; /** * An `@import` whose target is a bare string rather than a `url(...)`. diff --git a/tests/svg-active-content.test.mjs b/tests/svg-active-content.test.mjs index 5ba78e87..6d6e081f 100644 --- a/tests/svg-active-content.test.mjs +++ b/tests/svg-active-content.test.mjs @@ -1046,6 +1046,25 @@ test('the whitespace terminating a hex escape does not end a bare url token', () ]); }); +test('an unterminated escaped url() does not backtrack exponentially', () => { + // The bare-url alternatives must be mutually exclusive. When a hex run could + // split across them, `url(` followed by repeated escapes and never closed + // grew about 7x per repetition, so an upstream SVG could hang every job that + // scans one. 400 repetitions finished in under a second here and took tens + // of seconds at 10 repetitions before the fix. + const svg = + ''; + const started = Date.now(); + assert.deepEqual(findRemoteReferences(svg), []); + assert.ok( + Date.now() - started < 1000, + 'findRemoteReferences must stay linear on repeated unterminated escapes', + ); +}); + test('escaped separators still resolve to a protocol-relative target', () => { const svg = '