diff --git a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js index ba30a667..76c1011b 100644 --- a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js +++ b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js @@ -249,6 +249,17 @@ describe('sanitizeCSS', () => { expect(asParsed(sanitized)).not.toContain('evil.example'); }); + it.each([ + ['a leading escape', '.a{background:\\75rl(https://evil.example/p.gif)}'], + ['an escape mid-identifier', '.a{background:u\\72l(https://evil.example/p.gif)}'], + ])('blocks a url() spelled with %s', (_label, css) => { + // `\75` is the CSS escape for `u`, so a browser reads `\75rl(` as + // url(). The escape-outside-a-string handling has to run AFTER the + // identifier check, or it eats the escape and hides the token. + const { sanitized } = sanitizeCSS(css); + expect(asParsed(sanitized)).not.toContain('evil.example'); + }); + it('does not let an unterminated quote hide everything after it', () => { // An unclosed quote is a parse error. Trusting it meant one stray // apostrophe disabled scanning for the remainder of the stylesheet, so diff --git a/backend/src/utils/cssSanitizer.js b/backend/src/utils/cssSanitizer.js index 152e51c9..ed3623f5 100644 --- a/backend/src/utils/cssSanitizer.js +++ b/backend/src/utils/cssSanitizer.js @@ -140,18 +140,6 @@ function stripDisallowedUrls(css) { continue; } - // --- escape OUTSIDE a string ----------------------------------------- - // `\'` is an escaped identifier character, not the start of a string. - // Without this the scanner stepped onto the apostrophe, entered string - // mode, and copied the rest of the stylesheet unscanned — so - // `.hero{--marker:\';background:url(https://evil.example/p.gif)}` kept a - // live remote URL. Consume the escape and its escaped character together. - if (input[i] === '\\' && i + 1 < input.length) { - out += input.slice(i, i + 2); - i += 2; - continue; - } - // --- string ---------------------------------------------------------- // Escape-aware: `\"` inside a double-quoted string does NOT close it. // Decoding escapes up front (an earlier attempt) turned that into a real @@ -213,6 +201,19 @@ function stripDisallowedUrls(css) { continue; } + // --- escape that does NOT begin an identifier ------------------------- + // Ordered AFTER readIdentifier deliberately. `\75` is the escape for + // `u`, so `\75rl(...)` is url() to a browser — consuming the escape + // first would hide it from the check above, which is a bypass this + // branch introduced when it ran earlier. What is left for it is the + // `\'` case: an escaped quote that must not be read as opening a + // string, since that swallowed the rest of the stylesheet unscanned. + if (input[i] === '\\' && i + 1 < input.length) { + out += input.slice(i, i + 2); + i += 2; + continue; + } + out += input[i]; i += 1; }