From bf4a9f48eb8328446aa3db398f0aa6d3d035a1e3 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Sun, 12 Jul 2026 18:31:25 -0700
Subject: [PATCH] security: block data:image/svg+xml + sandbox HtmlBlock
 iframes

M-5: safeUrl() blocked javascript:/vbscript:/data:text/html but
allowed data:image/svg+xml, which can execute inline <script>/onload=
when loaded as a document/navigation target despite its "image" MIME
type (defense in depth -- not currently reachable to execution via
this sink, but closing it). Added `data:image/svg+xml` to the existing
DANGEROUS_SCHEME_PREFIXES check, so it's caught after the same
entity-decode/whitespace-strip/lowercase normalization used for the
other blocked schemes (obfuscated variants included). Other
data:image/* types (png/jpeg/gif/webp, ...) remain allowed unchanged.

M-6: HtmlBlock's purifyHtml() allowed <iframe src> through with no
`sandbox` attribute -- a clickjacking/phishing vector even with
DOMPurify already stripping script/on*=. Added a DOMPurify
afterSanitizeAttributes hook, scoped tightly to each purifyHtml() call
(added right before sanitize(), removed in a finally right after) so
it can't leak onto other DOMPurify uses or accumulate duplicates
across repeated calls, that force-sets a restrictive sandbox
(allow-scripts allow-same-origin allow-popups allow-forms -- no
allow-top-navigation) and referrerpolicy=no-referrer on every iframe
that survives sanitization.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 craft/src/components/basic/HtmlBlock.test.ts | 43 ++++++++++++++++++++
 craft/src/components/basic/HtmlBlock.tsx     | 30 +++++++++++++-
 craft/src/utils/escape.test.ts               | 18 ++++++++
 craft/src/utils/escape.ts                    |  9 +++-
 4 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/craft/src/components/basic/HtmlBlock.test.ts b/craft/src/components/basic/HtmlBlock.test.ts
index 86fa33d..4b9ebed 100644
--- a/craft/src/components/basic/HtmlBlock.test.ts
+++ b/craft/src/components/basic/HtmlBlock.test.ts
@@ -21,3 +21,46 @@ describe('purifyHtml', () => {
     expect(purifyHtml('<form><input name="x"></form>')).not.toContain('<form');
   });
 });
+
+describe('purifyHtml iframe sandboxing (M-6)', () => {
+  test('forces a restrictive sandbox attribute onto every iframe', () => {
+    const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
+    expect(out).toMatch(/<iframe[^>]*\bsandbox="[^"]+"/);
+  });
+
+  test('sandbox value omits allow-top-navigation (no top-level nav escape)', () => {
+    const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
+    const sandbox = out.match(/sandbox="([^"]*)"/)![1];
+    expect(sandbox).not.toMatch(/allow-top-navigation/);
+  });
+
+  test('legitimate embeds (YouTube) still work and get sandboxed too', () => {
+    const out = purifyHtml('<iframe src="https://www.youtube.com/embed/abc" allowfullscreen></iframe>');
+    expect(out).toContain('youtube.com/embed/abc');
+    expect(out).toMatch(/<iframe[^>]*\bsandbox="[^"]+"/);
+  });
+
+  test('adds referrerpolicy=no-referrer to iframes', () => {
+    const out = purifyHtml('<iframe src="https://example.com/"></iframe>');
+    expect(out).toContain('referrerpolicy="no-referrer"');
+  });
+
+  test('script/on* attributes are still stripped alongside the sandboxed iframe', () => {
+    const out = purifyHtml('<iframe src="https://example.com/" onload="alert(1)"></iframe><script>alert(2)</script>');
+    expect(out).not.toContain('onload');
+    expect(out).not.toContain('<script');
+  });
+
+  test('repeated calls do not leak/accumulate the hook (no duplicate sandbox attr, no cross-call state)', () => {
+    purifyHtml('<iframe src="https://a.example/"></iframe>');
+    purifyHtml('<iframe src="https://b.example/"></iframe>');
+    const out = purifyHtml('<iframe src="https://c.example/"></iframe>');
+    const sandboxMatches = out.match(/sandbox="/g) || [];
+    expect(sandboxMatches.length).toBe(1);
+  });
+
+  test('a non-iframe element sanitized alongside an iframe is not touched by the hook', () => {
+    const out = purifyHtml('<p>hi</p><iframe src="https://example.com/"></iframe>');
+    expect(out).toContain('<p>hi</p>');
+  });
+});
diff --git a/craft/src/components/basic/HtmlBlock.tsx b/craft/src/components/basic/HtmlBlock.tsx
index 4fa7f8a..0f41cc4 100644
--- a/craft/src/components/basic/HtmlBlock.tsx
+++ b/craft/src/components/basic/HtmlBlock.tsx
@@ -24,14 +24,42 @@ const PURIFY_CONFIG = {
     'href','src','alt','title','target','rel',
     'width','height','class',
     'allowfullscreen','allow','frameborder',
+    'sandbox','referrerpolicy',
   ],
   ALLOWED_URI_REGEXP: /^(?:(?:https?|mailto|tel|data:image\/[a-z]+;base64,):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i,
   FORBID_TAGS: ['script','style','object','embed','link','meta','form','input','button','select','textarea'],
   FORBID_ATTR: [/^on/i],
 };
 
+// M-6: `<iframe>` is allowed (maps/video embeds are a legitimate use case)
+// but an iframe with a `src` and NO `sandbox` attribute is a clickjacking/
+// phishing vector (DOMPurify already strips <script>/on*=, but an
+// unsandboxed iframe still gets full script execution, same-origin-ish
+// access via document.domain tricks, top-level navigation, etc., inside
+// itself). This hook force-sets a restrictive sandbox on every iframe that
+// survives sanitization, keeping `allow-scripts`/`allow-same-origin`/
+// `allow-popups`/`allow-forms` (needed for interactive maps/video/oauth
+// popups) but deliberately omitting `allow-top-navigation` so an embedded
+// page can never redirect/hijack the parent tab.
+const IFRAME_SANDBOX_HOOK = (node: Element): void => {
+  if (node.nodeName === 'IFRAME') {
+    node.setAttribute('sandbox', 'allow-scripts allow-same-origin allow-popups allow-forms');
+    node.setAttribute('referrerpolicy', 'no-referrer');
+  }
+};
+
 export function purifyHtml(input: string): string {
-  return DOMPurify.sanitize(input || '', PURIFY_CONFIG as any) as unknown as string;
+  // Hook is added immediately before sanitize() and removed immediately
+  // after, scoped tightly to this single call -- so it can never leak onto
+  // (or accumulate duplicate copies across) any other DOMPurify.sanitize()
+  // call elsewhere in the app, and repeated purifyHtml() calls never stack
+  // multiple copies of the same hook.
+  DOMPurify.addHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK);
+  try {
+    return DOMPurify.sanitize(input || '', PURIFY_CONFIG as any) as unknown as string;
+  } finally {
+    DOMPurify.removeHook('afterSanitizeAttributes', IFRAME_SANDBOX_HOOK as any);
+  }
 }
 
 export const HtmlBlock: UserComponent<HtmlBlockProps> = ({ code = '', style = {} }) => {
diff --git a/craft/src/utils/escape.test.ts b/craft/src/utils/escape.test.ts
index 27dffbd..5a93970 100644
--- a/craft/src/utils/escape.test.ts
+++ b/craft/src/utils/escape.test.ts
@@ -71,6 +71,24 @@ describe('safeUrl', () => {
     expect(safeUrl(s)).toBe(s);
   });
 
+  test('M-5: blocks data:image/svg+xml (can execute script when navigated to directly)', () => {
+    expect(safeUrl('data:image/svg+xml,<svg onload=alert(1)>')).toBe('');
+  });
+
+  test('M-5: blocks data:image/svg+xml;base64 variant', () => {
+    expect(safeUrl('data:image/svg+xml;base64,PHN2ZyBvbmxvYWQ9YWxlcnQoMSk+')).toBe('');
+  });
+
+  test('M-5: blocks obfuscated (whitespace/case) data:image/svg+xml', () => {
+    expect(safeUrl('  DATA:IMAGE/SVG+XML,<svg onload=alert(1)>')).toBe('');
+  });
+
+  test('M-5: still allows other data:image/* types unchanged', () => {
+    expect(safeUrl('data:image/jpeg;base64,/9j/4AAQ')).toBe('data:image/jpeg;base64,/9j/4AAQ');
+    expect(safeUrl('data:image/gif;base64,R0lGOD')).toBe('data:image/gif;base64,R0lGOD');
+    expect(safeUrl('data:image/webp;base64,UklGR')).toBe('data:image/webp;base64,UklGR');
+  });
+
   test.each([
     'https://x.com/a?b=1&c=2',
     'http://x',
diff --git a/craft/src/utils/escape.ts b/craft/src/utils/escape.ts
index 2e345b8..39bd953 100644
--- a/craft/src/utils/escape.ts
+++ b/craft/src/utils/escape.ts
@@ -36,7 +36,14 @@ export function escapeAttr(s: string): string {
 // Dangerous scheme prefixes, checked against a normalized copy with all
 // colons removed (see safeUrl) so that colon-splicing obfuscation like
 // "java:script:alert(1)" can't slip past a literal "javascript:" check.
-const DANGEROUS_SCHEME_PREFIXES = ['javascript', 'vbscript', 'datatext/html'];
+// M-5: `data:image/svg+xml` is blocked alongside `data:text/html` -- an SVG
+// document can carry an inline <script>/onload= just like an HTML document,
+// so it executes script when loaded as a document/navigation target (e.g. an
+// <a href> or window.open), even though it's nominally an "image" MIME type.
+// Other `data:image/*` types (png/jpeg/gif/webp, ...) stay allowed below --
+// only this specific scriptable subtype is blocked, regardless of a
+// trailing `;base64` or other parameters.
+const DANGEROUS_SCHEME_PREFIXES = ['javascript', 'vbscript', 'datatext/html', 'dataimage/svg+xml'];
 
 /** Decodes &#NN; and &#xNN; numeric HTML entities (used to obfuscate scheme names). */
 function decodeNumericEntities(s: string): string {