diff --git a/lib/__tests__/email-sanitization.test.ts b/lib/__tests__/email-sanitization.test.ts index 119fee224..42b23804f 100644 --- a/lib/__tests__/email-sanitization.test.ts +++ b/lib/__tests__/email-sanitization.test.ts @@ -841,5 +841,48 @@ describe('email-sanitization', () => { restrictDataUriResourcesOnNode(node); expect(node.getAttribute('src')).toBe(TRANSPARENT_BLOCKED_PIXEL); }); + + // DOMPurify URI-tests srcset as one string, so an allowed first candidate + // would wave an SVG second candidate through (#717 review finding). The + // guard re-checks every data: occurrence and drops the whole attribute. + it('drops a srcset that smuggles an svg candidate behind a raster one', () => { + const html = ''; + expect(sanitizeEmailHtmlForIframe(html)).not.toContain('svg'); + expect(sanitizeEmailHtml(html)).not.toContain('svg'); + }); + + it('drops a with a disallowed data: candidate', () => { + const html = ''; + const clean = sanitizeEmailHtmlForIframe(html); + expect(clean).not.toContain('svg'); + expect(clean).toContain('data:image/png;base64,AAAA'); + }); + + it('keeps an all-raster srcset and non-data candidates', () => { + const raster = ''; + expect(sanitizeEmailHtmlForIframe(raster)).toContain('srcset'); + const node = parseHtmlSafely('').querySelector('img')!; + restrictDataUriResourcesOnNode(node); + expect(node.getAttribute('srcset')).toContain('example.com/a.png'); + }); + + // The plain (non-base64) data: form ends its metadata with a comma; the + // srcset verdict must match the src verdict for the same URI. + it('keeps the comma-form raster data: URI a src would allow', () => { + const node = parseHtmlSafely('').querySelector('img')!; + restrictDataUriResourcesOnNode(node); + expect(node.getAttribute('srcset')).toContain('data:image/gif,R0lGODlh'); + }); + + // The external blocker stashes the whole srcset before this guard runs in + // the shared hook pass - the stash must not keep the disallowed candidate. + it('scrubs the data-blocked-srcset stash the external blocker leaves', () => { + const node = parseHtmlSafely('').querySelector('img')!; + blockExternalResourcesOnNode(node); + expect(node.getAttribute('data-blocked-srcset')).toContain('svg'); + restrictDataUriResourcesOnNode(node); + expect(node.hasAttribute('data-blocked-srcset')).toBe(false); + expect(node.hasAttribute('srcset')).toBe(false); + }); }); }); diff --git a/lib/email-sanitization.ts b/lib/email-sanitization.ts index 1aeb8d541..81106de9f 100644 --- a/lib/email-sanitization.ts +++ b/lib/email-sanitization.ts @@ -261,6 +261,25 @@ const DATA_URI_BYPASS_TAGS = new Set(['IMG', 'VIDEO', 'AUDIO', 'SOURCE', 'TRACK' const RASTER_IMAGE_DATA_URI = /^data:image\/(?:png|jpe?g|gif|webp|bmp|avif|x-icon|vnd\.microsoft\.icon)[;,]/i; +/** + * True if a `srcset` value carries a `data:` URI that isn't an allowed inline + * raster image. srcset candidates are comma-separated, but a data: URI itself + * contains commas, so the value can't be split into candidates reliably. + * Instead every `data:` occurrence is checked where it stands: its MIME type + * must be on the raster allowlist. The token is captured through the first + * comma so the plain form (`data:image/gif,...`) keeps the `[;,]` terminator + * the raster regexp requires - same verdict as the src path for the same URI. + * One disallowed hit condemns the whole attribute. No whitespace + * pre-normalization here: whitespace splits srcset tokens before the URL + * parser ever runs, so unlike in `src` it cannot hide a scheme inside a + * candidate. + */ +function srcsetHasDisallowedDataUri(srcset: string): boolean { + const dataUris = srcset.match(/data:[^,\s]*,?/gi); + if (!dataUris) return false; + return dataUris.some((uri) => !RASTER_IMAGE_DATA_URI.test(uri)); +} + /** * Drop `data:` URIs that aren't inline raster images from media elements. * DOMPurify cannot inspect the bytes inside a data: URI, so an SVG payload can @@ -283,6 +302,17 @@ export function restrictDataUriResourcesOnNode(node: Element): void { node.removeAttribute(attr); } } + // DOMPurify URI-tests srcset as one string, so an allowed first candidate + // waves every later one through - re-check per candidate. The external + // blocker's data-blocked-srcset stash gets the same treatment: it shares + // this hook pass with no ordering guarantee, and a stashed disallowed + // candidate must not outlive the live attribute. + for (const attr of ['srcset', 'data-blocked-srcset']) { + const value = node.getAttribute(attr); + if (value && srcsetHasDisallowedDataUri(value)) { + node.removeAttribute(attr); + } + } } /**