From a0a3c5243b081783d40b9d77b66c753b3eedc7dd Mon Sep 17 00:00:00 2001 From: sec-check Date: Sun, 4 Oct 2026 19:52:47 -0400 Subject: [PATCH] fix(security): treat backslash authority prefixes as remote references remoteTarget() accepted only `https?://` and a `//` prefix when deciding whether an SVG value points at another host. The URL parser treats `\` as `/` in the scheme and authority prefix of a special-scheme URL, and the site is served over https, so `\\host`, `/\host`, `\/host` and `https:\\host` all resolve to host exactly as `//host` does while the gate reported them local. findRemoteReferences() routes every href/src value and every url()/@import CSS target through remoteTarget(), so an imported diagram could reference an attacker-chosen host and still pass validate:architecture-assets. A visitor opening the published SVG then discloses their IP, User-Agent and Referer to that host -- the exposure the gate exists to prevent. Translate only the leading run of separators: a single separator keeps the value on this origin (`\host` is the path /host) and an interior backslash stays an ordinary path character, so `./sub\dir/x.png` is still local. Signed-off-by: sec-check --- scripts/lib/svg-active-content.mjs | 18 +++++++++++++-- tests/svg-active-content.test.mjs | 37 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/scripts/lib/svg-active-content.mjs b/scripts/lib/svg-active-content.mjs index 322bf53f..06cd0c3f 100644 --- a/scripts/lib/svg-active-content.mjs +++ b/scripts/lib/svg-active-content.mjs @@ -242,8 +242,22 @@ const CSS_IMPORT_PATTERN = /@import\s+(?:"([^"]*)"|'([^']*)')/gi; */ function remoteTarget(value) { const normalized = normalizeUri(value); - if (/^https?:\/\//.test(normalized) || normalized.startsWith('//')) { - return normalized; + // The URL parser treats `\` as `/` in the scheme and authority prefix of a + // special-scheme URL, and the site is served over https, so every relative + // reference resolves against a special-scheme base. `\\host`, `/\host`, + // `\/host` and `https:\\host` therefore reach `host` exactly as `//host` + // does, while a `//`-only test reads all four as same-origin paths. + // + // Only the leading run of separators is translated. A single separator + // keeps the value on this origin (`\host` is the path `/host`, and + // `https:/host` likewise), and an interior backslash is an ordinary path + // character -- `./sub\dir/x.png` must stay local, not become a host. + const authority = normalized.replace( + /^(https?:)?[/\\]{2,}/, + (match, scheme) => `${scheme ?? ''}//`, + ); + if (/^https?:\/\//.test(authority) || authority.startsWith('//')) { + return authority; } return null; } diff --git a/tests/svg-active-content.test.mjs b/tests/svg-active-content.test.mjs index 9efdcacf..3cf3e942 100644 --- a/tests/svg-active-content.test.mjs +++ b/tests/svg-active-content.test.mjs @@ -454,6 +454,43 @@ test('detects a protocol-relative reference, which inherits https at the origin' ]); }); +// The URL parser treats `\` as `/` in the scheme and authority prefix of a +// special-scheme URL, and the site is served over https. Each value below +// therefore reaches evil.example in a browser exactly as `//evil.example` +// does, so a `//`-only remote test reads a cross-origin fetch as a local path. +for (const [value, expected] of [ + ['\\\\evil.example/b.png', '//evil.example/b.png'], + ['/\\evil.example/b.png', '//evil.example/b.png'], + ['\\/evil.example/b.png', '//evil.example/b.png'], + ['\\\\\\evil.example/b.png', '//evil.example/b.png'], + ['https:\\\\evil.example/b.png', 'https://evil.example/b.png'], + ['https:/\\evil.example/b.png', 'https://evil.example/b.png'], + ['https:\\/evil.example/b.png', 'https://evil.example/b.png'], +]) { + test(`detects a backslash authority prefix: ${value}`, () => { + const svg = ``; + assert.deepEqual(findRemoteReferences(svg), [ + `references a remote resource in href: ${expected}`, + ]); + }); +} + +// Only the leading run of separators is an authority. A single separator keeps +// the value on this origin, and an interior backslash is an ordinary path +// character -- flagging either would fail diagrams that reference their own +// sibling assets. +for (const [label, value] of [ + ['a single leading backslash, which is a path on this origin', '\\local.png'], + ['a scheme with a single separator', 'https:/local.png'], + ['an interior backslash in a relative path', 'a\\b/c.png'], + ['a backslash inside a dot-relative path', './sub\\dir/x.png'], +]) { + test(`does not flag ${label}`, () => { + const svg = ``; + assert.deepEqual(findRemoteReferences(svg), []); + }); +} + test('detects a remote url() in a a';