diff --git a/CHANGELOG.md b/CHANGELOG.md index aa5172867d..1388744412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,21 @@ +# v2.0.4 (TBD) + +## Security Fixes 🔒 + +- **CRITICAL**: Fixed XSS vulnerability in HTML export feature + - Formula embeds now properly escape user-controlled values in `html()` output + - Video embeds now properly escape URLs in `html()` output + - Prevents script injection when using `getSemanticHTML()` or editor's HTML output + - Applications using "export HTML → store → render" workflows are now protected + - **Impact**: Malicious formulas or video URLs could execute JavaScript when exported HTML is rendered + - **Fix**: All special HTML characters (`<`, `>`, `&`, `"`, `'`) are now escaped in formula and video embed output + - **CVE**: Pending assignment + +## Additional Improvements + +- Improved HTML validity by properly escaping special characters in formulas and video URLs +- Added comprehensive XSS prevention test coverage for formula and video embeds + # v2.0.2 (2024-05-13) diff --git a/packages/quill/src/core/editor.ts b/packages/quill/src/core/editor.ts index b6391a7cd4..727298765d 100644 --- a/packages/quill/src/core/editor.ts +++ b/packages/quill/src/core/editor.ts @@ -399,13 +399,25 @@ function convertHTML( if (isRoot || blot.statics.blotName === 'list') { return parts.join(''); } - const { outerHTML, innerHTML } = blot.domNode as Element; - const [start, end] = outerHTML.split(`>${innerHTML}<`); + + // Safe HTML reconstruction - build tag with escaped attributes + const element = blot.domNode as Element; + const tagName = element.tagName.toLowerCase(); + const attributes = Array.from(element.attributes) + .map((attr) => { + // Sanitize attribute name to only allow valid characters + const safeName = attr.name.replace(/[^a-zA-Z0-9-_]/g, ''); + return `${safeName}="${escapeText(attr.value)}"`; + }) + .join(' '); + // TODO cleanup - if (start === '${parts.join('')}<${end}`; + if (tagName === 'table') { + return `${parts.join('')}
`; } - return `${start}>${parts.join('')}<${end}`; + + const openTag = attributes ? `<${tagName} ${attributes}>` : `<${tagName}>`; + return `${openTag}${parts.join('')}`; } return blot.domNode instanceof Element ? blot.domNode.outerHTML : ''; } diff --git a/packages/quill/src/formats/formula.ts b/packages/quill/src/formats/formula.ts index ded4e89160..dc8aab307c 100644 --- a/packages/quill/src/formats/formula.ts +++ b/packages/quill/src/formats/formula.ts @@ -1,4 +1,5 @@ import Embed from '../blots/embed.js'; +import { escapeText } from '../blots/text.js'; class Formula extends Embed { static blotName = 'formula'; @@ -26,9 +27,11 @@ class Formula extends Embed { return domNode.getAttribute('data-value'); } + domNode: HTMLElement; + html() { - const { formula } = this.value(); - return `${formula}`; + const formula = Formula.value(this.domNode) || ''; + return `${escapeText(formula)}`; } } diff --git a/packages/quill/src/formats/image.ts b/packages/quill/src/formats/image.ts index e68f56a0b3..564e57d022 100644 --- a/packages/quill/src/formats/image.ts +++ b/packages/quill/src/formats/image.ts @@ -1,5 +1,6 @@ import { EmbedBlot } from 'parchment'; import { sanitize } from './link.js'; +import { escapeText } from '../blots/text.js'; const ATTRIBUTES = ['alt', 'height', 'width']; @@ -52,6 +53,25 @@ class Image extends EmbedBlot { super.format(name, value); } } + + html() { + const { src, alt, width, height } = this.domNode; + const sanitizedSrc = Image.sanitize(src); + const sanitizedAlt = alt ? escapeText(alt) : ''; + + let attributes = `src="${escapeText(sanitizedSrc)}"`; + if (sanitizedAlt) { + attributes += ` alt="${sanitizedAlt}"`; + } + if (width) { + attributes += ` width="${escapeText(String(width))}"`; + } + if (height) { + attributes += ` height="${escapeText(String(height))}"`; + } + + return ``; + } } export default Image; diff --git a/packages/quill/src/formats/link.ts b/packages/quill/src/formats/link.ts index 5412355b85..5175c45f40 100644 --- a/packages/quill/src/formats/link.ts +++ b/packages/quill/src/formats/link.ts @@ -1,4 +1,5 @@ import Inline from '../blots/inline.js'; +import { escapeText } from '../blots/text.js'; class Link extends Inline { static blotName = 'link'; @@ -30,6 +31,39 @@ class Link extends Inline { this.domNode.setAttribute('href', this.constructor.sanitize(value)); } } + + html(index: number, length: number) { + // Re-sanitize href on export to prevent post-insertion DOM manipulation + // from injecting dangerous protocols (javascript:, data:, vbscript:) + const href = Link.sanitize(this.domNode.getAttribute('href') || ''); + const rel = this.domNode.getAttribute('rel'); + const target = this.domNode.getAttribute('target'); + + let attrs = `href="${escapeText(href)}"`; + if (rel) attrs += ` rel="${escapeText(rel)}"`; + if (target) attrs += ` target="${escapeText(target)}"`; + + // Render children safely via their own html() or escapeText fallback + const parts: string[] = []; + this.children.forEachAt(index, length, (child, offset, childLength) => { + if ('html' in child && typeof child.html === 'function') { + parts.push(child.html(offset, childLength)); + } else if ('value' in child && typeof child.value === 'function') { + // TextBlot path — escape the raw text value + parts.push( + escapeText( + (child.value() as string).slice(offset, offset + childLength), + ).replaceAll(' ', ' '), + ); + } else { + parts.push( + (child.domNode as Element).outerHTML, + ); + } + }); + + return `${parts.join('')}`; + } } function sanitize(url: string, protocols: string[]) { diff --git a/packages/quill/src/formats/video.ts b/packages/quill/src/formats/video.ts index 84d4bb15cf..716534be76 100644 --- a/packages/quill/src/formats/video.ts +++ b/packages/quill/src/formats/video.ts @@ -1,4 +1,5 @@ import { BlockEmbed } from '../blots/block.js'; +import { escapeText } from '../blots/text.js'; import Link from './link.js'; const ATTRIBUTES = ['height', 'width']; @@ -51,8 +52,8 @@ class Video extends BlockEmbed { } html() { - const { video } = this.value(); - return `${video}`; + const video = Video.value(this.domNode) || ''; + return `${escapeText(video)}`; } } diff --git a/packages/quill/src/modules/syntax.ts b/packages/quill/src/modules/syntax.ts index da99fdc41d..5a16588075 100644 --- a/packages/quill/src/modules/syntax.ts +++ b/packages/quill/src/modules/syntax.ts @@ -159,7 +159,7 @@ class SyntaxCodeBlockContainer extends CodeBlockContainer { ? SyntaxCodeBlock.formats(codeBlock.domNode) : 'plain'; - return `
\n${escapeText(
+    return `
\n${escapeText(
       this.code(index, length),
     )}\n
`;
   }
diff --git a/packages/quill/src/themes/snow.ts b/packages/quill/src/themes/snow.ts
index 153a834c26..df0c984c44 100644
--- a/packages/quill/src/themes/snow.ts
+++ b/packages/quill/src/themes/snow.ts
@@ -66,10 +66,12 @@ class SnowTooltip extends BaseTooltip {
           if (link != null) {
             this.linkRange = new Range(range.index - offset, link.length());
             const preview = LinkBlot.formats(link.domNode);
+            // Re-sanitize the link before displaying in preview
+            const sanitizedPreview = preview ? LinkBlot.sanitize(preview) : '';
             // @ts-expect-error Fix me later
-            this.preview.textContent = preview;
+            this.preview.textContent = sanitizedPreview;
             // @ts-expect-error Fix me later
-            this.preview.setAttribute('href', preview);
+            this.preview.setAttribute('href', sanitizedPreview);
             this.show();
             const bounds = this.quill.getBounds(this.linkRange);
             if (bounds != null) {
diff --git a/packages/quill/test/unit/core/editor-xss.spec.ts b/packages/quill/test/unit/core/editor-xss.spec.ts
new file mode 100644
index 0000000000..a1e7e7bb52
--- /dev/null
+++ b/packages/quill/test/unit/core/editor-xss.spec.ts
@@ -0,0 +1,200 @@
+import { describe, expect, test } from 'vitest';
+import { createRegistry } from '../__helpers__/factory.js';
+import Quill from '../../../src/core.js';
+import { normalizeHTML } from '../__helpers__/utils.js';
+import List, { ListContainer } from '../../../src/formats/list.js';
+import Bold from '../../../src/formats/bold.js';
+import Link from '../../../src/formats/link.js';
+import Italic from '../../../src/formats/italic.js';
+
+const createEditor = (htmlOrContents: string) => {
+  const container = document.createElement('div');
+  container.innerHTML = normalizeHTML(htmlOrContents);
+  document.body.appendChild(container);
+  const quill = new Quill(container, {
+    registry: createRegistry([
+      ListContainer,
+      List,
+      Bold,
+      Link,
+      Italic,
+    ]),
+  });
+  return quill.editor;
+};
+
+describe('Editor XSS Prevention (convertHTML)', () => {
+  describe('Attribute Escaping', () => {
+    test('escapes quotes in element attributes', () => {
+      const editor = createEditor(
+        '
Link
',
+      );
+      const html = editor.getHTML(0, 5);
+
+      // Should escape quotes to prevent attribute injection
+      expect(html).toContain('"');
+      // Should NOT allow onclick attribute injection
+      expect(html).not.toContain('" onclick="');
+    });
+
+    test('escapes ampersands in attributes', () => {
+      const editor = createEditor(
+        '
Link
',
+      );
+      const html = editor.getHTML(0, 5);
+
+      // Should preserve escaped ampersands
+      expect(html).toContain('&');
+    });
+
+    test('escapes less than and greater than in attributes', () => {
+      const editor = createEditor(
+        '
Link
',
+      );
+      const html = editor.getHTML(0, 5);
+
+      // Should escape < and >
+      expect(html).toContain('<');
+      expect(html).toContain('>');
+    });
+
+    test('prevents script injection via attributes', () => {
+      const editor = createEditor(
+        '
Link
',
+      );
+      const html = editor.getHTML(0, 5);
+
+      // Should NOT contain unescaped script tags
+      expect(html).not.toContain('';
+      editor.insertEmbed(0, 'formula', malicious);
+      const html = editor.getHTML(0, 2);
+
+      // Should NOT contain unescaped HTML
+      expect(html).not.toContain('');
+
+      // Should contain escaped version
+      expect(html).toContain('<script>');
+      expect(html).toContain('</script>');
+    });
+
+    test('escapes malicious closing tags', () => {
+      const editor = new Editor(createScroll('

'));
+      const malicious = '';
+      editor.insertEmbed(0, 'formula', malicious);
+      const html = editor.getHTML(0, 2);
+
+      // Should NOT contain unescaped closing tag or img tag
+      expect(html).not.toContain(' {
+      const editor = new Editor(createScroll('

'));
+      const malicious = '" onload="alert(1)';
+      editor.insertEmbed(0, 'formula', malicious);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain escaped quotes
+      expect(html).toContain('"');
+      // The word "onload" might appear but should be escaped, prevent the actual exploit
+      expect(html).not.toContain('" onload="');
+    });
+
+    test('escapes ampersands in formula', () => {
+      const editor = new Editor(createScroll('

'));
+      const formula = 'a & b';
+      editor.insertEmbed(0, 'formula', formula);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain escaped ampersand
+      expect(html).toContain('&');
+    });
+
+    test('escapes less than and greater than operators', () => {
+      const editor = new Editor(createScroll('

'));
+      const formula = 'x < 5 && y > 3';
+      editor.insertEmbed(0, 'formula', formula);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain escaped operators (this also fixes invalid HTML)
+      expect(html).toContain('<');
+      expect(html).toContain('>');
+      expect(html).toContain('&&');
+    });
+
+    test('handles normal formulas without special characters', () => {
+      const editor = new Editor(createScroll('

'));
+      const formula = 'E=mc^2';
+      editor.insertEmbed(0, 'formula', formula);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain the formula as-is (no special chars to escape)
+      expect(html).toContain('E=mc^2');
+    });
+
+    test('handles empty formula', () => {
+      const editor = new Editor(createScroll('

'));
+      editor.insertEmbed(0, 'formula', '');
+      const html = editor.getHTML(0, 2);
+
+      // Should create valid HTML even with empty formula
+      expect(html).toContain('');
+    });
+
+    test('prevents double-escaping of already-escaped content', () => {
+      const editor = new Editor(createScroll('

'));
+      // User explicitly enters escaped content
+      const alreadyEscaped = '<script>';
+      editor.insertEmbed(0, 'formula', alreadyEscaped);
+      const html = editor.getHTML(0, 2);
+
+      // Should double-escape (correct behavior - user wanted literal text)
+      expect(html).toContain('&lt;script&gt;');
+    });
+  });
+});
+
diff --git a/packages/quill/test/unit/formats/image.spec.ts b/packages/quill/test/unit/formats/image.spec.ts
new file mode 100644
index 0000000000..be5686d375
--- /dev/null
+++ b/packages/quill/test/unit/formats/image.spec.ts
@@ -0,0 +1,225 @@
+import { describe, expect, test } from 'vitest';
+import {
+  createScroll as baseCreateScroll,
+  createRegistry,
+} from '../__helpers__/factory.js';
+import Editor from '../../../src/core/editor.js';
+import Image from '../../../src/formats/image.js';
+
+const createScroll = (html: string) =>
+  baseCreateScroll(html, createRegistry([Image]));
+
+describe('Image', () => {
+  describe('XSS Prevention', () => {
+    test('prevents onerror attribute injection', () => {
+      // This is the critical vulnerability: img tags with onerror handlers
+      const scroll = createScroll(
+        '
',
+      );
+      const imageEditor = new Editor(scroll);
+      const html = imageEditor.getHTML(0, 2);
+
+      // Should NOT contain the onerror attribute
+      expect(html).not.toContain('onerror');
+      expect(html).not.toContain('alert(1)');
+      expect(html).not.toContain('alert');
+      
+      // Should only contain safe attributes
+      expect(html).toContain(' {
+      const scroll = createScroll(
+        '
',
+      );
+      const editor = new Editor(scroll);
+      const html = editor.getHTML(0, 2);
+
+      // Should NOT contain the onclick attribute
+      expect(html).not.toContain('onclick');
+      expect(html).not.toContain('alert');
+    });
+
+    test('prevents onload attribute injection', () => {
+      const scroll = createScroll(
+        '
',
+      );
+      const editor = new Editor(scroll);
+      const html = editor.getHTML(0, 2);
+
+      // Should NOT contain the onload attribute
+      expect(html).not.toContain('onload');
+      expect(html).not.toContain('alert');
+    });
+
+    test('escapes quotes in alt attribute', () => {
+      const scroll = createScroll(
+        '
test" onerror="alert(1)
',
+      );
+      const editor = new Editor(scroll);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain escaped quotes
+      expect(html).toContain('"');
+      // Should NOT allow attribute injection
+      expect(html).not.toContain('" onerror="');
+    });
+
+    test('escapes HTML in alt attribute', () => {
+      const scroll = createScroll(
+        '
<script>alert(1)</script>
',
+      );
+      const editor = new Editor(scroll);
+      const html = editor.getHTML(0, 2);
+
+      // Should contain double-escaped content (escaped once in DOM, escaped again in output)
+      expect(html).not.toContain('');
+
+    const html = editor.getHTML(0, 5);
+
+    expect(html).not.toContain('data:text/html');
+    expect(html).toContain('href="about:blank"');
+  });
+
+  test('sanitizes vbscript: protocol injected after insertion', () => {
+    const editor = new Editor(createScroll('
click
'));
+    editor.formatText(0, 5, { link: 'https://safe.com' });
+
+    const anchor = editor.scroll.domNode.querySelector('a')!;
+    anchor.setAttribute('href', 'vbscript:MsgBox(1)');
+
+    const html = editor.getHTML(0, 5);
+
+    expect(html).not.toContain('vbscript:');
+    expect(html).toContain('href="about:blank"');
+  });
+
+  test('preserves safe http: links unchanged', () => {
+    const editor = new Editor(createScroll('
link
'));
+    editor.formatText(0, 4, { link: 'http://example.com/path?a=1&b=2' });
+
+    const html = editor.getHTML(0, 4);
+
+    expect(html).toContain('href="http://example.com/path?a=1&b=2"');
+    expect(html).not.toContain('javascript:');
+  });
+
+  test('preserves safe https: links unchanged', () => {
+    const editor = new Editor(createScroll('
link
'));
+    editor.formatText(0, 4, { link: 'https://quilljs.com' });
+
+    const html = editor.getHTML(0, 4);
+
+    expect(html).toContain('href="https://quilljs.com"');
+  });
+
+  test('preserves mailto: links unchanged', () => {
+    const editor = new Editor(createScroll('
email
'));
+    editor.formatText(0, 5, { link: 'mailto:user@example.com' });
+
+    const html = editor.getHTML(0, 5);
+
+    expect(html).toContain('href="mailto:user@example.com"');
+  });
+
+  test('escapes special characters in href attribute value', () => {
+    const editor = new Editor(createScroll('
link
'));
+    editor.formatText(0, 4, { link: 'https://example.com?a=1&b=<2>' });
+
+    const html = editor.getHTML(0, 4);
+
+    // & and < must be HTML-entity-escaped inside an attribute
+    expect(html).toContain('&');
+    expect(html).toContain('<');
+    expect(html).not.toContain('href="https://example.com?a=1&b=<2>"');
+  });
+
+  test('escapes quotes injected into href to prevent attribute breakout', () => {
+    const editor = new Editor(createScroll('
link
'));
+    editor.formatText(0, 4, { link: 'https://safe.com' });
+
+    const anchor = editor.scroll.domNode.querySelector('a')!;
+    anchor.setAttribute('href', 'https://safe.com" onclick="alert(1)');
+
+    const html = editor.getHTML(0, 4);
+
+    expect(html).not.toContain('" onclick="');
+    expect(html).toContain('"');
+  });
+
+  test('includes rel and target attributes safely', () => {
+    const editor = new Editor(createScroll('
link
'));
+    editor.formatText(0, 4, { link: 'https://example.com' });
+
+    const html = editor.getHTML(0, 4);
+
+    expect(html).toContain('rel="noopener noreferrer"');
+    expect(html).toContain('target="_blank"');
+  });
+
+  test('renders link text content safely (not innerHTML)', () => {
+    const editor = new Editor(createScroll('
click here
'));
+    editor.formatText(0, 10, { link: 'https://example.com' });
+
+    const html = editor.getHTML(0, 10);
+
+    // Link text should be present and safely rendered
+    expect(html).toContain('>click here');
+    expect(html).toContain('href="https://example.com"');
+  });
+});
diff --git a/packages/quill/test/unit/formats/video.spec.ts b/packages/quill/test/unit/formats/video.spec.ts
new file mode 100644
index 0000000000..068d4bae86
--- /dev/null
+++ b/packages/quill/test/unit/formats/video.spec.ts
@@ -0,0 +1,84 @@
+import { describe, expect, test } from 'vitest';
+import {
+  createScroll as baseCreateScroll,
+  createRegistry,
+} from '../__helpers__/factory.js';
+import Editor from '../../../src/core/editor.js';
+import Video from '../../../src/formats/video.js';
+
+const createScroll = (html: string) =>
+  baseCreateScroll(html, createRegistry([Video]));
+
+describe('Video', () => {
+  describe('XSS Prevention', () => {
+    test('escapes HTML tags in video URL', () => {
+      const editor = new Editor(createScroll('

'));
+      const malicious = '';
+      editor.insertEmbed(0, 'video', malicious);
+      const html = editor.getHTML(0, 2);
+
+      // Should NOT contain unescaped HTML
+      expect(html).not.toContain('');
+
+      // Should contain escaped version
+      expect(html).toContain('<script>');
+      expect(html).toContain('</script>');
+    });
+
+    test('escapes malicious attributes in video URL', () => {
+      const editor = new Editor(createScroll('

'));
+      const malicious = '">');
+    });
+  });
+});
diff --git a/packages/quill/test/unit/modules/syntax.spec.ts b/packages/quill/test/unit/modules/syntax.spec.ts
index cd1646bbb6..51b5111bb8 100644
--- a/packages/quill/test/unit/modules/syntax.spec.ts
+++ b/packages/quill/test/unit/modules/syntax.spec.ts
@@ -292,4 +292,152 @@ describe('Syntax', () => {
       expect(quill.getSemanticHTML()).toContain('data-language="javascript"');
     });
   });
+
+  describe('XSS Prevention', () => {
+    test('escapes quotes in data-language attribute', () => {
+      const container = document.body.appendChild(
+        document.createElement('div'),
+      );
+      container.innerHTML = normalizeHTML(
+        `

`,
+      );
+      const quill = new Quill(container, {
+        modules: {
+          syntax: {
+            hljs,
+            interval: HIGHLIGHT_INTERVAL,
+          },
+        },
+        registry: createRegistry([
+          CodeToken,
+          CodeBlock,
+          Quill.import('formats/code-block-container'),
+        ]),
+      });
+      const html = quill.getSemanticHTML();
+
+      // Should escape quotes
+      expect(html).toContain('"');
+      // Should NOT allow attribute injection
+      expect(html).not.toContain('" onclick="');
+    });
+
+    test('escapes malicious language closing tag', () => {
+      const container = document.body.appendChild(
+        document.createElement('div'),
+      );
+      container.innerHTML = normalizeHTML(
+        `

`,
+      );
+      const quill = new Quill(container, {
+        modules: {
+          syntax: {
+            hljs,
+            interval: HIGHLIGHT_INTERVAL,
+          },
+        },
+        registry: createRegistry([
+          CodeToken,
+          CodeBlock,
+          Quill.import('formats/code-block-container'),
+        ]),
+      });
+      const html = quill.getSemanticHTML();
+
+      // Should NOT contain unescaped script tags
+      expect(html).not.toContain('