From d30ec4a19db9307b8a1997d0a71675e8480eb884 Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Tue, 7 Apr 2026 20:34:39 +0500 Subject: [PATCH] core: make html validation more resilient --- packages/core/src/api/sync/merger.ts | 2 +- .../src/utils/__tests__/html-parser.test.ts | 160 +++++++++++++++++- packages/core/src/utils/dom-purify.ts | 19 ++- packages/core/src/utils/html-parser.ts | 35 +++- 4 files changed, 204 insertions(+), 12 deletions(-) diff --git a/packages/core/src/api/sync/merger.ts b/packages/core/src/api/sync/merger.ts index 0ac222341..4cbffa450 100644 --- a/packages/core/src/api/sync/merger.ts +++ b/packages/core/src/api/sync/merger.ts @@ -215,7 +215,7 @@ export async function handleInboxItems( JSON.parse(decryptedItem) ); if (!validation.success) { - logger.info("Failed to validate inbox item.", { + logger.warn("Failed to validate inbox item.", { inboxItem: item, errors: validation.error.issues }); diff --git a/packages/core/src/utils/__tests__/html-parser.test.ts b/packages/core/src/utils/__tests__/html-parser.test.ts index 4ffe935d4..b30fe685d 100644 --- a/packages/core/src/utils/__tests__/html-parser.test.ts +++ b/packages/core/src/utils/__tests__/html-parser.test.ts @@ -16,7 +16,7 @@ GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see . */ -import { normalizeToHtmlBody } from "../html-parser.js"; +import { normalizeToHtmlBody, sanitizeHtml } from "../html-parser.js"; import { expect, describe, it } from "vitest"; const HTML_INPUT_TYPES: Array<{ @@ -139,3 +139,161 @@ describe("normalizeToHtmlBody", () => { ); }); }); + +// sanitizeHtml uses globalThis.DOMParser (set to linkedom's DOMParser in +// test.setup.ts) to back DOMPurify when a native browser DOM is unavailable. +describe("sanitizeHtml", () => { + it("strips "); + expect(result).not.toContain(" { + const result = sanitizeHtml(''); + expect(result).not.toContain("onerror"); + expect(result).not.toContain("alert(1)"); + }); + + it("strips javascript: URIs from href", () => { + // eslint-disable-next-line no-script-url + const result = sanitizeHtml('click'); + expect(result).not.toContain("javascript:"); + expect(result).toContain("click"); + }); + + it("strips javascript: URIs from src", () => { + // eslint-disable-next-line no-script-url + const result = sanitizeHtml( + '' + ); + expect(result).not.toContain("javascript:"); + }); + + it("strips onclick and other on* attributes", () => { + const result = sanitizeHtml( + '
x
' + ); + expect(result).not.toContain("onclick"); + expect(result).not.toContain("onmouseover"); + expect(result).not.toContain("evil()"); + }); + + it("strips and tags", () => { + const result = sanitizeHtml( + '' + ); + expect(result).not.toContain(" { + const result = sanitizeHtml( + 'x' + ); + expect(result).not.toMatch(/href=["']data:/i); + }); + + it("preserves safe block elements", () => { + const input = "

Hello world

  • item
"; + const result = sanitizeHtml(input); + expect(result).toContain("

"); + expect(result).toContain("world"); + expect(result).toContain("

    "); + expect(result).toContain("
  • item
    • "); + }); + + it("preserves safe links with http/https href", () => { + const result = sanitizeHtml('Notes'); + expect(result).toContain('href="https://notesnook.com"'); + expect(result).toContain("Notes"); + }); + + it("preserves headings", () => { + const result = sanitizeHtml("

    Title

    Subtitle

    "); + expect(result).toContain("

    Title

    "); + expect(result).toContain("

    Subtitle

    "); + }); + + it("returns a string (not TrustedHTML or DOM node)", () => { + const result = sanitizeHtml("

    test

    "); + expect(typeof result).toBe("string"); + }); + + it("handles empty input without throwing", () => { + expect(() => sanitizeHtml("")).not.toThrow(); + const result = sanitizeHtml(""); + expect(typeof result).toBe("string"); + }); + + it("handles plain text without throwing", () => { + const result = sanitizeHtml("just plain text"); + expect(result).toContain("just plain text"); + expect(typeof result).toBe("string"); + }); + + it("handles deeply nested XSS attempts", () => { + const result = sanitizeHtml( + "

    hover

    " + ); + expect(result).not.toContain("onmouseover"); + expect(result).toContain("hover"); + }); + + it("strips tag that could hijack relative URLs", () => { + const result = sanitizeHtml( + 'link' + ); + expect(result).not.toContain(" with safe https src", () => { + const result = sanitizeHtml(''); + expect(result).toContain(" with javascript: URI", () => { + // eslint-disable-next-line no-script-url + const result = sanitizeHtml( + '' + ); + expect(result).toContain(" with data: URI", () => { + const result = sanitizeHtml( + '' + ); + expect(result).toContain("", () => { + const result = sanitizeHtml( + '' + ); + expect(result).toContain("", () => { + const result = sanitizeHtml( + '' + ); + expect(result).toContain(" with safe src alongside other elements", () => { + const result = sanitizeHtml( + '

    Safe content

    ' + ); + expect(result).toContain("Safe content"); + expect(result).toContain("dompurify" ); const win = document?.defaultView as any; + if (!win) + throw new Error("DOMParser is unavailable; cannot initialize DOMPurify"); const doc = win.document; // DOMPurify calls new DOMParser().parseFromString(...) @@ -92,9 +94,22 @@ function createPurifyWindow() { let domPurify: DOMPurify.DOMPurify | undefined = undefined; function getDomPurify() { - if (DOMPurify.isSupported) return DOMPurify; if (!domPurify) { - domPurify = DOMPurify(createPurifyWindow()); + const win = DOMPurify.isSupported + ? (globalThis as unknown as WindowLike) + : createPurifyWindow(); + domPurify = DOMPurify(win); + // Only allow https/http src on iframes — strip anything else (javascript:, + // data:, relative paths that could be weaponized, etc.). + domPurify.addHook("uponSanitizeAttribute", (node, data) => { + if ( + node.tagName === "IFRAME" && + data.attrName === "src" && + !/^https?:\/\//i.test(data.attrValue) + ) { + data.keepAttr = false; + } + }); } return domPurify; } diff --git a/packages/core/src/utils/html-parser.ts b/packages/core/src/utils/html-parser.ts index 9d385edb2..ebb99c630 100644 --- a/packages/core/src/utils/html-parser.ts +++ b/packages/core/src/utils/html-parser.ts @@ -29,9 +29,12 @@ export const parseHTML = (input: string) => ) : null; -export const sanitizeHtml = (html: string) => { +export const sanitizeHtml = (html: string): string => { const inputHtml = normalizeToHtmlBody(html); - return getDomPurify().sanitize(inputHtml); + return getDomPurify().sanitize(inputHtml, { + RETURN_DOM: false, + ADD_TAGS: ["iframe"] + }) as string; }; export function getDummyDocument() { @@ -71,9 +74,16 @@ function isHtmlValid(html: string): boolean { const trimmed = html.trim(); if (!trimmed) return true; + // Strip comments and script/style content before tag counting to avoid + // false matches on tags appearing inside comments or raw text blocks. + const stripped = trimmed + .replace(//g, "") + .replace(/]*>[\s\S]*?<\/script>/gi, "") + .replace(/]*>[\s\S]*?<\/style>/gi, ""); + // Extract all opening and closing tag names - const openTagMatches = trimmed.matchAll(/<([a-z][a-z0-9]*)\b/gi); - const closeTagMatches = trimmed.matchAll(/<\/([a-z][a-z0-9]*)\b/gi); + const openTagMatches = stripped.matchAll(/<([a-z][a-z0-9]*)\b/gi); + const closeTagMatches = stripped.matchAll(/<\/([a-z][a-z0-9]*)\b/gi); const openTags = Array.from(openTagMatches, (m) => m[1].toLowerCase()); const closeTags = Array.from(closeTagMatches, (m) => m[1].toLowerCase()); @@ -81,9 +91,14 @@ function isHtmlValid(html: string): boolean { // Document-level tags (body, html, head) are allowed to be unclosed in fragments const documentTags = new Set(["body", "html", "head"]); - // Count content tags (non-document tags) - const openContentTags = openTags.filter((tag) => !documentTags.has(tag)); - const closeContentTags = closeTags.filter((tag) => !documentTags.has(tag)); + // Count content tags (non-document, non-void tags) — void/self-closing elements + // never have a closing tag so they must not affect the balance check. + const openContentTags = openTags.filter( + (tag) => !documentTags.has(tag) && !SELF_CLOSING_TAGS.has(tag) + ); + const closeContentTags = closeTags.filter( + (tag) => !documentTags.has(tag) && !SELF_CLOSING_TAGS.has(tag) + ); // For content tags: opening and closing must match if (openContentTags.length !== closeContentTags.length) { @@ -103,6 +118,10 @@ function isHtmlValid(html: string): boolean { }, onclosetag: (name) => { const nameLower = name.toLowerCase(); + // htmlparser2 fires onclosetag for void/self-closing elements immediately + // after onopentag. We never push them onto the stack, so skip here too. + if (SELF_CLOSING_TAGS.has(nameLower)) return; + const lastOpen = openStack[openStack.length - 1]; if (!lastOpen) { @@ -177,7 +196,7 @@ export function normalizeToHtmlBody(input: string) { } if (headBlock) { - const bodyContent = inner.replace(headBlock!, "").trim(); + const bodyContent = inner.replace(headBlock, "").trim(); return `${headBlock}${bodyContent}`; }