hi
` + ); + expect(out).not.toContain('evil.example'); + }); + + it('blocks a tracking url() hidden behind an escaped quote', () => { + const out = sanitizeCampaignBody( + `hi
` + ); + expect(out).not.toContain('evil.example'); + }); + + it('keeps a legitimate quoted font stack, re-encoded for the attribute', () => { + const out = sanitizeCampaignBody( + `hi
` + ); + expect(out).toContain('color:red'); + expect(out).toContain('Helvetica Neue'); + // Re-encoded, so the attribute stays well formed rather than being cut short. + expect(out).not.toMatch(/style="[^"]*"[^>]*"/); + }); + it('strips expression() out of an inline style', () => { const out = sanitizeCampaignBody('hi
'); expect(out).not.toContain('expression('); diff --git a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js index 8863b151..ba30a667 100644 --- a/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js +++ b/backend/__tests__/utils/cssSanitizer.remoteUrls.test.js @@ -239,6 +239,33 @@ describe('sanitizeCSS', () => { expect(warnings.join(' ')).toContain('external URL'); }); + it('blocks a url() hidden behind an escaped quote outside a string', () => { + // `\'` is an escaped identifier character, not a string opener. The + // scanner used to step onto the apostrophe, enter string mode, and copy + // the rest of the stylesheet — url() included — unexamined. + const { sanitized } = sanitizeCSS( + ".hero{--marker:\\';background:url(https://evil.example/p.gif)}" + ); + 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 + // the safe reading is to treat it as an ordinary character and continue. + const { sanitized } = sanitizeCSS( + "p{font-family:'don't;background:url(https://evil.example/p.gif)}" + ); + expect(asParsed(sanitized)).not.toContain('evil.example'); + }); + + it('still keeps legitimate quoted values and data: images intact', () => { + const font = sanitizeCSS('p{font-family:"Helvetica Neue",sans-serif;color:red}'); + expect(font.sanitized).toContain('"Helvetica Neue"'); + const data = sanitizeCSS(".a{background:url('data:image/png;base64,iVBORw0KGgo=')}"); + expect(data.sanitized).toContain('data:image/png'); + }); + it('still blocks the other forbidden patterns', () => { const { sanitized } = sanitizeCSS( '@import url("https://x/e.css"); .a{width:expression(alert(1));behavior:url(e.htc)}' diff --git a/backend/src/services/newsletterService.js b/backend/src/services/newsletterService.js index eaec1f76..8a29f79a 100644 --- a/backend/src/services/newsletterService.js +++ b/backend/src/services/newsletterService.js @@ -145,12 +145,52 @@ function sanitizeCampaignBody(html) { // rather than pattern-matching it, so the local pass this used to need is // gone. Keeping a second copy would mean two definitions of "disallowed" // drifting apart. + // + // Entities are decoded BEFORE the CSS is scanned, and re-encoded after. + // sanitize-html emits `"` inside an attribute as `"`, so the scanner + // and the recipient's browser otherwise disagree about where CSS strings + // begin: in `style="font-family:"don't";background:url(...)"` + // the browser decodes first and reads the apostrophe as ordinary text + // inside a real string, then makes the url() request — while the scanner + // saw the apostrophe open a string and skipped everything after it. The + // scanner has to be shown what the browser will actually parse. .replace(/style="([^"]*)"/gi, (match, css) => { - const { sanitized } = sanitizeCSS(css); - return sanitized ? `style="${sanitized.replace(/"/g, '')}"` : ''; + const { sanitized } = sanitizeCSS(decodeHtmlEntities(css)); + return sanitized ? `style="${encodeForAttribute(sanitized)}"` : ''; }); } +/** + * Decode the HTML entities sanitize-html emits inside attribute values, so + * CSS is scanned in the form the recipient's parser will see. One pass, so + * `"` decodes to `"` and not to `"`. + */ +function decodeHtmlEntities(value) { + return String(value).replace( + /&(?:#(\d+)|#[xX]([0-9a-fA-F]+)|(quot|apos|amp|lt|gt));/g, + (whole, dec, hex, name) => { + if (dec !== undefined) { + const code = Number(dec); + return code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : whole; + } + if (hex !== undefined) { + const code = parseInt(hex, 16); + return code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : whole; + } + return { quot: '"', apos: '\'', amp: '&', lt: '<', gt: '>' }[name]; + } + ); +} + +/** Re-encode a sanitized value so it is safe inside a double-quoted attribute. */ +function encodeForAttribute(value) { + return String(value) + .replace(/&/g, '&') + .replace(/"/g, '"') + .replace(//g, '>'); +} + /** * Sanitize a campaign's optional `