Remove by default unsafe attribute values from HTML. Fixes #5743

GrapesJS · Mar 14, 2024 · 1101af3 · 1101af3
1 parent a54f213
commit 1101af3
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 8 deletions.
diff --git a/src/parser/config/config.ts b/src/parser/config/config.ts
@@ -32,11 +32,25 @@ export interface HTMLParserOptions {
    */
   allowUnsafeAttr?: boolean;
 
+  /**
+   * Allow unsafe HTML attribute values (eg. `src="javascript:..."`).
+   * @default false
+   */
+  allowUnsafeAttrValue?: boolean;
+
   /**
    * When false, removes empty text nodes when parsed, unless they contain a space.
    * @default false
    */
   keepEmptyTextNodes?: boolean;
+
+  /**
+   * Custom transformer to run before passing the input HTML to the parser.
+   * A common use case might be to sanitize the input string.
+   * @example
+   * preParser: htmlString => DOMPurify.sanitize(htmlString)
+   */
+  preParser?: (input: string, opts: { editor: Editor }) => string;
 }
 
 export interface ParserConfig {
@@ -84,6 +98,7 @@ const config: ParserConfig = {
     htmlType: 'text/html',
     allowScripts: false,
     allowUnsafeAttr: false,
+    allowUnsafeAttrValue: false,
     keepEmptyTextNodes: false,
   },
 };

diff --git a/src/parser/model/ParserHtml.ts b/src/parser/model/ParserHtml.ts
@@ -310,7 +310,9 @@ const ParserHtml = (em?: EditorModel, config: ParserConfig & { returnArray?: boo
         htmlType: config.optionsHtml?.htmlType || config.htmlType,
         ...opts,
       };
-      const el = isFunction(cf.parserHtml) ? cf.parserHtml(str, options) : BrowserParserHtml(str, options);
+      const { preParser } = options;
+      const input = isFunction(preParser) ? preParser(str, { editor: em?.getEditor()! }) : str;
+      const el = isFunction(cf.parserHtml) ? cf.parserHtml(input, options) : BrowserParserHtml(input, options);
       const scripts = el.querySelectorAll('script');
       let i = scripts.length;
 
@@ -323,8 +325,8 @@ const ParserHtml = (em?: EditorModel, config: ParserConfig & { returnArray?: boo
       }
 
       // Remove unsafe attributes
-      if (!options.allowUnsafeAttr) {
-        this.__clearUnsafeAttr(el);
+      if (!options.allowUnsafeAttr || !options.allowUnsafeAttrValue) {
+        this.__sanitizeNode(el, options);
       }
 
       // Detach style tags and parse them
@@ -341,26 +343,28 @@ const ParserHtml = (em?: EditorModel, config: ParserConfig & { returnArray?: boo
         if (styleStr) res.css = parserCss.parse(styleStr);
       }
 
-      em && em.trigger(`${event}:root`, { input: str, root: el });
+      em?.trigger(`${event}:root`, { input, root: el });
       const result = this.parseNode(el, cf);
       // I have to keep it otherwise it breaks the DomComponents.addComponent (returns always array)
       const resHtml = result.length === 1 && !cf.returnArray ? result[0] : result;
       res.html = resHtml;
-      em && em.trigger(event, { input: str, output: res });
+      em?.trigger(event, { input, output: res });
 
       return res;
     },
 
-    __clearUnsafeAttr(node: HTMLElement) {
+    __sanitizeNode(node: HTMLElement, opts: HTMLParserOptions) {
       const attrs = node.attributes || [];
       const nodes = node.childNodes || [];
       const toRemove: string[] = [];
       each(attrs, attr => {
         const name = attr.nodeName || '';
-        name.indexOf('on') === 0 && toRemove.push(name);
+        const value = attr.nodeValue || '';
+        !opts.allowUnsafeAttr && name.startsWith('on') && toRemove.push(name);
+        !opts.allowUnsafeAttrValue && value.startsWith('javascript:') && toRemove.push(name);
       });
       toRemove.map(name => node.removeAttribute(name));
-      each(nodes, node => this.__clearUnsafeAttr(node as HTMLElement));
+      each(nodes, node => this.__sanitizeNode(node as HTMLElement, opts));
     },
   };
 };

diff --git a/test/specs/parser/model/ParserHtml.ts b/test/specs/parser/model/ParserHtml.ts
@@ -600,4 +600,59 @@ describe('ParserHtml', () => {
     ];
     expect(obj.parse(str).html).toEqual(result);
   });
+
+  describe('Options', () => {
+    test('Remove unsafe attributes', () => {
+      const str = '<img src="path/img" data-test="1" class="test" onload="unsafe"/>';
+      const result = {
+        type: 'image',
+        tagName: 'img',
+        classes: ['test'],
+        attributes: {
+          src: 'path/img',
+          'data-test': '1',
+        },
+      };
+      expect(obj.parse(str).html).toEqual([result]);
+      expect(obj.parse(str, null, { allowUnsafeAttr: true }).html).toEqual([
+        {
+          ...result,
+          attributes: {
+            ...result.attributes,
+            onload: 'unsafe',
+          },
+        },
+      ]);
+    });
+
+    test('Remove unsafe attribute values', () => {
+      const str = '<iframe src="javascript:alert(1)"></iframe>';
+      const result = {
+        type: 'iframe',
+        tagName: 'iframe',
+      };
+      expect(obj.parse(str).html).toEqual([result]);
+      expect(obj.parse(str, null, { allowUnsafeAttrValue: true }).html).toEqual([
+        {
+          ...result,
+          attributes: {
+            src: 'javascript:alert(1)',
+          },
+        },
+      ]);
+    });
+
+    test('Custom preParser option', () => {
+      const str = '<iframe src="javascript:alert(1)"></iframe>';
+      const result = {
+        type: 'iframe',
+        tagName: 'iframe',
+        attributes: {
+          src: 'test:alert(1)',
+        },
+      };
+      const preParser = (str: string) => str.replace('javascript:', 'test:');
+      expect(obj.parse(str, null, { preParser }).html).toEqual([result]);
+    });
+  });
 });