fix(site-builder): repair dead-code data:image clause in HTML block URI regex
Review follow-up on the Task 24 sanitiser widening (approved, no bypass found). Two Important findings to close: 1. ALLOWED_URI_REGEXP's data:image/...;base64, arm sat inside the group that appends a trailing `:` to every alternative, so it required a second colon no real data URI has -- the clause could never match. Confirmed dead before the fix (poster/cite/href all stripped a valid base64 PNG data URI) and working after (all three now survive), while javascript:/data:text/html stay blocked. Pulled the arm out into its own top-level alternative. 2. Corrected an inaccurate comment/report claim that every allowed attribute value goes through this regex -- `src` on img/video/audio/source/image/track is additionally covered by DOMPurify's own DATA_URI_TAGS allow-list, which is mimetype-blind and bypasses the regex entirely (acceptable: none of those tags execute src as a document; iframe is correctly excluded from that list). Adds two regression tests: the regex fix actually working, and the DATA_URI_TAGS bypass pinned so a future DOMPurify change surfaces as a failing test rather than a surprise. Fixture byte figures unchanged (15,815 -> 14,899; fixture has no data:image URIs). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -257,4 +257,43 @@ describe('purifyHtml -- Task 24: security properties on newly-allowed elements',
|
|||||||
);
|
);
|
||||||
expect(out).not.toContain('data:text/html');
|
expect(out).not.toContain('data:text/html');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('review fix: data:image/*;base64, URIs now actually survive on poster/cite/href (dead-code regex bug)', () => {
|
||||||
|
// ALLOWED_URI_REGEXP used to put the data:image arm inside the group
|
||||||
|
// that gets a trailing `:` appended to every alternative, requiring a
|
||||||
|
// second colon after the one already in "base64," -- which no real
|
||||||
|
// data URI has, so the clause could never match anything. Confirm the
|
||||||
|
// fixed regex actually allows a real base64 image data URI through on
|
||||||
|
// ordinary URI-checked attributes (not just the DATA_URI_TAGS-covered
|
||||||
|
// src ones tested below).
|
||||||
|
const b64 = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=';
|
||||||
|
const out = purifyHtml(
|
||||||
|
`<video poster="data:image/png;base64,${b64}"></video>` +
|
||||||
|
`<blockquote cite="data:image/png;base64,${b64}">q</blockquote>` +
|
||||||
|
`<a href="data:image/png;base64,${b64}">img</a>`,
|
||||||
|
);
|
||||||
|
expect(out).toContain(`poster="data:image/png;base64,${b64}"`);
|
||||||
|
expect(out).toContain(`cite="data:image/png;base64,${b64}"`);
|
||||||
|
expect(out).toContain(`href="data:image/png;base64,${b64}"`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('documented reality: data: on img/video/audio/source src is mimetype-blind (DOMPurify DATA_URI_TAGS bypasses ALLOWED_URI_REGEXP)', () => {
|
||||||
|
// This is NOT gated by ALLOWED_URI_REGEXP at all -- DOMPurify has its
|
||||||
|
// own internal DATA_URI_TAGS allow-list (img, video, audio, source,
|
||||||
|
// image, track) that accepts ANY data: URI on the `src` attribute of
|
||||||
|
// those tags regardless of declared mimetype, before our regex is ever
|
||||||
|
// consulted. Acceptable because none of those tags execute their src
|
||||||
|
// as a document/script context in mainstream browsers -- the sink
|
||||||
|
// doesn't execute. Pinned here so a future DOMPurify version change to
|
||||||
|
// DATA_URI_TAGS shows up as a failing test, not a surprise in
|
||||||
|
// production. <iframe> -- the one tag where this WOULD be dangerous --
|
||||||
|
// is correctly not in DOMPurify's DATA_URI_TAGS list, so its src still
|
||||||
|
// goes through the normal ALLOWED_URI_REGEXP check and gets stripped.
|
||||||
|
const b64 = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=';
|
||||||
|
const imgOut = purifyHtml(`<img src="data:text/html;base64,${b64}">`);
|
||||||
|
expect(imgOut).toContain(`src="data:text/html;base64,${b64}"`);
|
||||||
|
|
||||||
|
const iframeOut = purifyHtml(`<iframe src="data:text/html;base64,${b64}"></iframe>`);
|
||||||
|
expect(iframeOut).not.toContain('data:');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -41,11 +41,18 @@ const PURIFY_CONFIG = {
|
|||||||
// on*= handlers specifically to prove they still get neutralized/
|
// on*= handlers specifically to prove they still get neutralized/
|
||||||
// dropped by staying outside the allow-list.
|
// dropped by staying outside the allow-list.
|
||||||
'details','summary',
|
'details','summary',
|
||||||
// Media (Task 24). All URL-bearing attributes on these (src, poster,
|
// Media (Task 24). URL-bearing attributes on these (poster, srcset,
|
||||||
// srcset...) go through the same ALLOWED_URI_REGEXP gate as everything
|
// action, cite...) go through the ALLOWED_URI_REGEXP gate like
|
||||||
// else -- see _isValidAttribute in dompurify, which URI-checks every
|
// everything else -- see _isValidAttribute in dompurify, which
|
||||||
// allowed attribute value except a small fixed "inert" list (alt,
|
// URI-checks every allowed attribute value except a small fixed
|
||||||
// class, id, style, title, ...) that never includes src/poster/srcset.
|
// "inert" list (alt, class, id, style, title, ...) that never includes
|
||||||
|
// src/poster/srcset. The one exception: `src` itself on img/video/
|
||||||
|
// audio/source/image/track is additionally covered by DOMPurify's own
|
||||||
|
// `DATA_URI_TAGS` allow-list, which accepts any data: URI on those
|
||||||
|
// tag/attribute pairs regardless of mimetype, bypassing this regex --
|
||||||
|
// see the ALLOWED_URI_REGEXP comment below and
|
||||||
|
// HtmlBlock.security.test.ts. Not a gap in the four non-negotiables:
|
||||||
|
// none of those tags execute their src as a document.
|
||||||
'picture','source','video','audio','track','canvas',
|
'picture','source','video','audio','track','canvas',
|
||||||
// Forms (Task 24). Site owner's explicit decision: allow the full
|
// Forms (Task 24). Site owner's explicit decision: allow the full
|
||||||
// ordinary form surface. No on*= survives (FORBID_ATTR below), and
|
// ordinary form surface. No on*= survives (FORBID_ATTR below), and
|
||||||
@@ -98,7 +105,20 @@ const PURIFY_CONFIG = {
|
|||||||
'stop-color','stop-opacity','gradientunits','gradienttransform',
|
'stop-color','stop-opacity','gradientunits','gradienttransform',
|
||||||
'preserveaspectratio',
|
'preserveaspectratio',
|
||||||
],
|
],
|
||||||
ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto|tel|data:image\/[a-z]+;base64,):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i,
|
// Review fix (Task 24 follow-up): the data:image arm used to sit inside
|
||||||
|
// the group that gets a trailing `:` appended for every alternative
|
||||||
|
// (`(?:https?|mailto|tel|data:image\/[a-z]+;base64,):`), so it required
|
||||||
|
// a SECOND colon after the one already in "base64,figure" -- no real
|
||||||
|
// data URI has that, so the clause could never match. It is now its own
|
||||||
|
// top-level alternative. NOTE: this regex is not the only thing gating
|
||||||
|
// data: URIs -- DOMPurify has its own internal `DATA_URI_TAGS` allow-list
|
||||||
|
// (img/video/audio/source/image/track) that accepts ANY data: URI on
|
||||||
|
// those tag/attribute pairs regardless of declared mimetype, bypassing
|
||||||
|
// this regex entirely. See HtmlBlock.security.test.ts for a regression
|
||||||
|
// test documenting that (acceptable: none of those tags execute their
|
||||||
|
// src as a document in mainstream browsers, and <iframe> -- which would
|
||||||
|
// be dangerous -- is correctly not in that DOMPurify list).
|
||||||
|
ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto|tel):|data:image\/[a-z]+;base64,|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i,
|
||||||
// form/input/button/select/textarea removed from FORBID_TAGS (Task 24) --
|
// form/input/button/select/textarea removed from FORBID_TAGS (Task 24) --
|
||||||
// they are now deliberately allowed above. style/script/object/embed/
|
// they are now deliberately allowed above. style/script/object/embed/
|
||||||
// link/meta stay forbidden; <style> in particular stays blocked even
|
// link/meta stay forbidden; <style> in particular stays blocked even
|
||||||
|
|||||||
Reference in New Issue
Block a user