From 2b5cb66ee246dad274ce832e545c56b967752a4d Mon Sep 17 00:00:00 2001 From: Mario Heiderich Date: Tue, 23 Jun 2026 12:18:34 +0200 Subject: [PATCH 1/3] test: added better test coverage for non-innerHTML sinks --- README.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index bf7d207..aa0c079 100644 --- a/README.md +++ b/README.md @@ -293,11 +293,19 @@ It's a retrofit, not magic. Know the edges (the - **Load it first.** Whoever registers the `default` policy first wins. If attacker code beats you to it, you're worse off than before. Don't add `'allow-duplicates'`. - **One realm at a time.** Each iframe is its own world and needs its own DOMFortify. -- **Trusted Types sinks only.** DOMFortify sanitizes the Trusted Types HTML sinks. Other sinks - `style` - and CSS injection, `javascript:` URLs, and inline handlers - sit outside that contract, and their - behavior under enforcement varies by browser. Close them definitively with a real CSP alongside the - Trusted Types one, for example `script-src 'self'; object-src 'none'; base-uri 'none'` (no - `'unsafe-inline'`). +- **Trusted Types sinks only.** DOMFortify covers exactly the Trusted Types sinks, and inside that + contract it is thorough. Every HTML sink (`innerHTML`, `outerHTML`, `insertAdjacentHTML`, + `document.write`, `Range.createContextualFragment`, `iframe.srcdoc`) is sanitized, so inline event + handlers and `javascript:` URLs that arrive as markup are stripped; every string-to-code sink + (`eval`, `Function`, string `setTimeout`/`setInterval`, `script.text`, `script.src`, Worker URLs) is + refused; and `setAttribute('onclick', ...)` is refused too, since the browser treats event-handler + content attributes as a TrustedScript sink. What sits outside the contract is only the residue no + Trusted Types policy can see: assigning a function to a handler property (`el.onclick = fn`, which + already presupposes script execution and is not reachable by markup injection), assigning a + `javascript:` URL straight to a property (`a.href = '...'`), and `style`/CSS injection. Close those + definitively with a real CSP alongside the Trusted Types one, for example + `script-src 'self'; object-src 'none'; base-uri 'none'` (no `'unsafe-inline'`). This boundary is + pinned by the `sink-boundary` e2e matrix. - **One sanitizer.** A bypass in the sanitizer is a bypass in everything it guards. - **It sanitizes a string, then the sink re-parses it.** The `default` policy returns sanitized HTML as a string that the browser parses again in context - the serialize/re-parse step that can re-open From 352cebfc72cafb5d4a6e8480b67830bb68e18924 Mon Sep 17 00:00:00 2001 From: Mario Heiderich Date: Tue, 23 Jun 2026 12:20:04 +0200 Subject: [PATCH 2/3] test: added actual test fixtures --- test/e2e/sink-boundary.spec.ts | 82 ++++++++++++++++++++ test/fixtures/sink-boundary-unprotected.html | 12 +++ test/fixtures/sink-boundary.html | 17 ++++ test/fixtures/sink-boundary.runner.js | 52 +++++++++++++ 4 files changed, 163 insertions(+) create mode 100644 test/e2e/sink-boundary.spec.ts create mode 100644 test/fixtures/sink-boundary-unprotected.html create mode 100644 test/fixtures/sink-boundary.html create mode 100644 test/fixtures/sink-boundary.runner.js diff --git a/test/e2e/sink-boundary.spec.ts b/test/e2e/sink-boundary.spec.ts new file mode 100644 index 0000000..6dc15e5 --- /dev/null +++ b/test/e2e/sink-boundary.spec.ts @@ -0,0 +1,82 @@ +/** + * Sink-boundary coverage matrix. + * + * Proves, vector by vector, exactly where DOMFortify's protection begins and ends, so a future change + * cannot silently reopen a sink. Every vector runs in genuine page context inside the fixture (see the + * note in sink-boundary.runner.js for why this must not move into page.evaluate); the spec only reads + * the recorded result after a short settle that lets async sinks (string setTimeout, script.src) land. + * + * COVERED: must be neutralized under DOMFortify, and must execute on the unprotected canary (which + * proves each vector is a real, working sink and that the detector sees it). + * BOUNDARY: outside the Trusted Types contract by design (function-handler assignment); documented, + * not guarded as a vulnerability. + */ +import { test, expect, type Page } from '@playwright/test'; + +const COVERED = [ + 'innerHTML', + 'outerHTML', + 'insertAdjacentHTML', + 'createContextualFragment', + 'template.innerHTML', + 'eval', + 'Function', + 'setTimeout(string)', + 'script.text', + 'script.src', + 'setAttribute-onclick', +]; +const BOUNDARY = ['el.onclick = fn']; + +interface Probe { + status: Record | null; + fired: Record; + matrix: { label: string; category: string; threw: boolean; msg: string }[]; +} + +async function probe(page: Page, fixture: string): Promise { + await page.goto(`/test/fixtures/${fixture}`); + await page.waitForFunction(() => (window as unknown as { __matrixReady?: boolean }).__matrixReady === true, null, { + timeout: 5_000, + }); + await page.waitForTimeout(300); // let async sinks (string setTimeout, script.src) settle + return page.evaluate(() => ({ + status: + (window as unknown as { DOMFortify?: { status?: () => Record | null } }).DOMFortify?.status?.() ?? + null, + fired: (window as unknown as { __fired: Record }).__fired, + matrix: (window as unknown as { __matrix: Probe['matrix'] }).__matrix, + })); +} + +// --- Canary: on an unprotected page every covered vector must execute ----------------------------- +test('canary: every covered sink executes on the unprotected page', async ({ page }) => { + const { fired } = await probe(page, 'sink-boundary-unprotected.html'); + for (const label of COVERED) { + expect(fired[label], `${label} must execute when nothing is enforcing, or the test is meaningless`).toBe(true); + } +}); + +// --- Protected: every covered vector is neutralized ----------------------------------------------- +test('every covered sink is neutralized under DOMFortify', async ({ page }) => { + const { status, fired } = await probe(page, 'sink-boundary.html'); + // Only assert the guarantee where the engine actually enforces Trusted Types (matches the rest of + // the e2e suite); non-enforcing engine builds skip rather than fail. + test.skip(!status?.enforcementActive, 'engine build does not enforce Trusted Types natively'); + expect(status?.protected, 'page should report protected').toBe(true); + for (const label of COVERED) { + expect(fired[label] ?? false, `${label} must be neutralized under enforcement`).toBe(false); + } +}); + +// --- Boundary: function-handler assignment is outside the contract and stays so -------------------- +test('boundary sinks remain outside the Trusted Types contract (documented, not guarded)', async ({ page }) => { + const { status, fired } = await probe(page, 'sink-boundary.html'); + test.skip(!status?.enforcementActive, 'engine build does not enforce Trusted Types natively'); + for (const label of BOUNDARY) { + expect( + fired[label] ?? false, + `${label} sits outside Trusted Types; if this changes, re-evaluate the threat model and docs`, + ).toBe(true); + } +}); diff --git a/test/fixtures/sink-boundary-unprotected.html b/test/fixtures/sink-boundary-unprotected.html new file mode 100644 index 0000000..0be9e72 --- /dev/null +++ b/test/fixtures/sink-boundary-unprotected.html @@ -0,0 +1,12 @@ + + + + + + +
+ + + diff --git a/test/fixtures/sink-boundary.html b/test/fixtures/sink-boundary.html new file mode 100644 index 0000000..7d12fc6 --- /dev/null +++ b/test/fixtures/sink-boundary.html @@ -0,0 +1,17 @@ + + + + + + + + + + +
+ + + diff --git a/test/fixtures/sink-boundary.runner.js b/test/fixtures/sink-boundary.runner.js new file mode 100644 index 0000000..9aa5544 --- /dev/null +++ b/test/fixtures/sink-boundary.runner.js @@ -0,0 +1,52 @@ +/** + * Sink-boundary matrix runner (shared by the protected fixture and the unprotected canary). + * + * It runs every vector in GENUINE page context - this file executes as the page loads, NOT through + * Playwright's page.evaluate. That is deliberate and load-bearing: page.evaluate runs in a CDP context + * that bypasses Trusted Types enforcement for eval()/Function() specifically (innerHTML and setTimeout + * stay gated), so measuring those from page.evaluate falsely reports them as executed. Driving the + * sinks from the page itself is the only honest way to test eval/Function. Do not "simplify" this into + * page.evaluate. + * + * Each payload calls window.__H('