diff --git a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js index be8c39fe..30d1e035 100644 --- a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js +++ b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js @@ -260,6 +260,17 @@ describe('sanitizeCSS', () => { expect(asParsed(sanitized)).not.toContain('evil.example'); }); + it('blocks a url() the TAG strip would have un-quoted', () => { + // `<[^>]*>` deletes the span it matches, and `<">` takes a quote with it. + // Running that after the URL scan meant the scanner saw the url() safely + // inside a string and this pass then removed the quotes that made it so. + // URL validation has to be the last thing that looks at the text. + const { sanitized } = sanitizeCSS( + '--x:x<">;background:url(https://evil.example/p.gif);--y:x<">' + ); + expect(asParsed(sanitized)).not.toContain('evil.example'); + }); + it('does not treat NBSP as CSS whitespace inside url()', () => { // JS `\s` matches U+00A0; CSS whitespace does not. Skipping it let the // scanner read the following quote as a legitimate data: URI and swallow diff --git a/backend/src/utils/cssSanitizer.js b/backend/src/utils/cssSanitizer.js index 98df28ea..b8780d68 100644 --- a/backend/src/utils/cssSanitizer.js +++ b/backend/src/utils/cssSanitizer.js @@ -345,6 +345,19 @@ function sanitizeCSS(cssContent) { // below it then produced exactly the request the scan was there to // prevent. Any pass that can join tokens has to happen before validation, // not after. + // Remove any remaining script-like content. This is the LAST pass that can + // move text, and so it must run before the URL scan, not after: it deletes + // the matched span, and a span like `<">` takes a quote with it. That is + // how `--x:x<">;background:url(https://evil.example/p.gif);--y:x<">` shipped + // a live background — the scanner saw the url() safely inside a string, and + // this line then removed the quotes that made it so. + sanitized = sanitized.replace(/<[^>]*>/g, '/* BLOCKED TAG */'); + + // URL validation runs LAST, deliberately. Every pass above rewrites the + // text, and each one that did so after this point has produced a bypass: + // the HTML-comment strip (#1290), the control-character strip, and the tag + // strip immediately above. Validating anything other than the final bytes + // means validating a string that is not the one that gets served. const urlPass = stripDisallowedUrls(sanitized); if (urlPass.blocked > 0) { warnings.push( @@ -354,9 +367,6 @@ function sanitizeCSS(cssContent) { sanitized = urlPass.sanitized; } - // Remove any remaining script-like content - sanitized = sanitized.replace(/<[^>]*>/g, '/* BLOCKED TAG */'); - return { sanitized: sanitized.trim(), warnings }; }