From 34072c7189406bc51fb0fd8a5c6fe5624c5be0cb Mon Sep 17 00:00:00 2001 From: Irshad Ahamed M B Date: Fri, 16 Oct 2026 12:17:28 +0530 Subject: [PATCH 1/3] fix(editor): sanitize HTML output from getHTML/convertHTML to prevent script injection (Snyk report) --- packages/quill/src/core/editor.ts | 129 ++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 8 deletions(-) diff --git a/packages/quill/src/core/editor.ts b/packages/quill/src/core/editor.ts index b6391a7cd4..74f12df300 100644 --- a/packages/quill/src/core/editor.ts +++ b/packages/quill/src/core/editor.ts @@ -360,6 +360,72 @@ function convertListHTML( return `${convertListHTML(items, lastIndent - 1, types)}`; } +/* + Security: sanitize HTML returned from editor to avoid injecting scripts or unsafe attributes. + We avoid returning unsanitized outerHTML from DOM nodes. Instead we reconstruct elements + with a safe subset of attributes and validate URLs to prevent javascript: URIs. +*/ + +function isSafeUrl(url: string | null | undefined) { + if (!url) return false; + const trimmed = url.trim().toLowerCase(); + // Disallow javascript: and data: URIs that could run scripts. Allow http(s) and mailto and relative URLs + return ( + trimmed.startsWith('http://') || + trimmed.startsWith('https://') || + trimmed.startsWith('/') || + trimmed.startsWith('#') || + trimmed.startsWith('mailto:') || + // Allow protocol-relative + trimmed.startsWith('//') + ); +} + +function sanitizeAttributes(el: Element) { + const allowedAttrs = new Set([ + 'href', + 'src', + 'alt', + 'title', + 'width', + 'height', + 'colspan', + 'rowspan', + 'data-list', + 'data-id', + ]); + const attrs: string[] = []; + for (let i = 0; i < el.attributes.length; i++) { + const a = el.attributes[i]; + const name = a.name.toLowerCase(); + const value = a.value; + // Drop event handlers and styles + if (name.startsWith('on') || name === 'style') continue; + // Only allow whitelisted attributes + if (!allowedAttrs.has(name) && !name.startsWith('data-')) continue; + // Validate URLs + if ((name === 'href' || name === 'src') && !isSafeUrl(value)) continue; + // Escape attribute value + const escaped = String(value) + .replace(/&/g, '&') + .replace(/\"/g, '"') + .replace(//g, '>'); + attrs.push(`${name}="${escaped}"`); + } + return attrs.length ? ' ' + attrs.join(' ') : ''; +} + +function serializeElementSafely(el: Element, innerHtml: string) { + const tag = el.tagName.toLowerCase(); + const attrs = sanitizeAttributes(el); + // For tables, preserve a simple visible border without allowing arbitrary style attributes + if (tag === 'table') { + return `${innerHtml}`; + } + return `<${tag}${attrs}>${innerHtml}`; +} + function convertHTML( blot: Blot, index: number, @@ -367,7 +433,48 @@ function convertHTML( isRoot = false, ): string { if ('html' in blot && typeof blot.html === 'function') { - return blot.html(index, length); + // If blot provides its own html, sanitize the returned string + try { + const raw = blot.html(index, length); + // Simple sanitize: parse into a template and reconstruct allowed parts + const container = document.createElement('template'); + container.innerHTML = raw || ''; + // Walk and remove disallowed elements (script, iframe, object, embed) + const forbidden = ['script', 'iframe', 'object', 'embed', 'style']; + forbidden.forEach((tag) => { + container.content.querySelectorAll(tag).forEach((node) => node.remove()); + }); + // For remaining elements, strip unsafe attributes + container.content.querySelectorAll('*').forEach((node) => { + if (node instanceof Element) { + const attrs = sanitizeAttributes(node); + // Rebuild node with allowed attributes only + // Create new node to avoid keeping original attributes + const newNode = document.createElement(node.tagName.toLowerCase()); + if (attrs) { + // attrs string is like ' name=\"value\" ...' — set via innerHTML trick + const dummy = document.createElement('div'); + dummy.innerHTML = `<${node.tagName.toLowerCase()}${attrs}>`; + const newEl = dummy.firstChild as Element; + if (newEl && newEl.attributes) { + for (let i = 0; i < newEl.attributes.length; i++) { + const a = newEl.attributes[i]; + newNode.setAttribute(a.name, a.value); + } + } + } + // Move children + while (node.firstChild) { + newNode.appendChild(node.firstChild); + } + node.replaceWith(newNode); + } + }); + return (container.innerHTML || '').replaceAll(' ', ' '); + } catch (e) { + // If blot.html throws or DOM not available, fall back to safe empty string + return ''; + } } if (blot instanceof TextBlot) { const escapedText = escapeText(blot.value().slice(index, index + length)); @@ -399,15 +506,21 @@ function convertHTML( if (isRoot || blot.statics.blotName === 'list') { return parts.join(''); } - const { outerHTML, innerHTML } = blot.domNode as Element; - const [start, end] = outerHTML.split(`>${innerHTML}<`); - // TODO cleanup - if (start === '${parts.join('')}<${end}`; + // Reconstruct element safely instead of using outerHTML/innerHTML which may contain unsafe attributes + const dom = blot.domNode as Element; + const inner = parts.join(''); + return serializeElementSafely(dom, inner); + } + if (blot.domNode instanceof Element) { + // Non-Parent blots: return a safe serialization of the element + try { + const dom = blot.domNode as Element; + return serializeElementSafely(dom, dom.textContent ? escapeText(dom.textContent) : ''); + } catch (e) { + return ''; } - return `${start}>${parts.join('')}<${end}`; } - return blot.domNode instanceof Element ? blot.domNode.outerHTML : ''; + return ''; } function combineFormats( From 6ee70341928f7de3342e1e4392105ddaec529fdb Mon Sep 17 00:00:00 2001 From: Irshad Ahamed M B Date: Fri, 16 Oct 2026 12:20:19 +0530 Subject: [PATCH 2/3] chore: PR description file for fix/sanitize-getHTML --- PR_DESCRIPTION.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md new file mode 100644 index 0000000000..c8f6867e38 --- /dev/null +++ b/PR_DESCRIPTION.md @@ -0,0 +1,21 @@ +Title: fix(editor): sanitize HTML output from getHTML/convertHTML to prevent script injection (Snyk report) + +Summary: +Sanitize HTML produced by Editor.getHTML/convertHTML to remove scriptable elements and unsafe attributes/URLs. This addresses a Snyk-reported injection risk by: + +- Removing forbidden elements (script, iframe, object, embed, style) returned by blots. +- Stripping event handlers and style attributes. +- Whitelisting attributes (href, src, alt, title, width, height, colspan, rowspan, data-*). +- Validating href/src values to disallow javascript: and unsafe data: URIs. +- Reconstructing DOM elements safely rather than returning outerHTML/innerHTML verbatim. + +Tests: +- Build & unit tests: yarn build && yarn test +- Manual checks: insert malicious markup (script, iframe, onclick handlers, style URLs, javascript: links) and verify editor.getHTML() strips or neutralizes them. Verify normal links/images/lists/tables still serialize correctly. + +Security rationale: +- Removes executable elements and event handlers. +- Validates URL schemes and disallows javascript/data URIs. +- Escapes attribute values to avoid injection via attributes. + +Patch location: packages/quill/src/core/editor.ts From 021e169f253d111fbe7a3547bdc0d5de81d605dc Mon Sep 17 00:00:00 2001 From: Irshad Ahamed M B Date: Fri, 16 Oct 2026 12:25:11 +0530 Subject: [PATCH 3/3] Delete PR_DESCRIPTION.md --- PR_DESCRIPTION.md | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md deleted file mode 100644 index c8f6867e38..0000000000 --- a/PR_DESCRIPTION.md +++ /dev/null @@ -1,21 +0,0 @@ -Title: fix(editor): sanitize HTML output from getHTML/convertHTML to prevent script injection (Snyk report) - -Summary: -Sanitize HTML produced by Editor.getHTML/convertHTML to remove scriptable elements and unsafe attributes/URLs. This addresses a Snyk-reported injection risk by: - -- Removing forbidden elements (script, iframe, object, embed, style) returned by blots. -- Stripping event handlers and style attributes. -- Whitelisting attributes (href, src, alt, title, width, height, colspan, rowspan, data-*). -- Validating href/src values to disallow javascript: and unsafe data: URIs. -- Reconstructing DOM elements safely rather than returning outerHTML/innerHTML verbatim. - -Tests: -- Build & unit tests: yarn build && yarn test -- Manual checks: insert malicious markup (script, iframe, onclick handlers, style URLs, javascript: links) and verify editor.getHTML() strips or neutralizes them. Verify normal links/images/lists/tables still serialize correctly. - -Security rationale: -- Removes executable elements and event handlers. -- Validates URL schemes and disallows javascript/data URIs. -- Escapes attribute values to avoid injection via attributes. - -Patch location: packages/quill/src/core/editor.ts