Cross Site Scripting (XSS) via Markdown

[snoopysecurity]

I'm submitting a bug report

• Library Version: 2.0.2

• Browser: all | Chrome | Firefox

Current behavior: The editor doesn’t do any sort of sanitization of inserted HTML and markdown and this could result in Cross-site Scripting (XSS). An introduction to XSS can be read here: https://excess-xss.com

Expected/desired behavior: I think the best way to fix this would be to use the following JavaScript package https://www.npmjs.com/package/dompurify. DomPurify is XSS sanitizer and is a good solution to prevent XSS. An example of a markdown editor maintainer discussing usage of this can be seen here: https://github.com/nhn/tui.editor/issues/733.

• What is the expected behavior? Steps to Reproduce:
  1. Go to https://ghiscoding.github.io/angular-markdown-editor/#/reactive-editor
  2. Insert any of the following payloads:

     ![Uh oh...](https://www.example.com/image.png"onload="alert('XSS'))
     ![Uh oh...]("onerror="alert('XSS'))
     [Click Me](javascript:alert('Uh oh...'))

• What is the motivation / use case for changing the behavior? Security feature

[ghiscoding]

I use DOMPurify in my other library (Angular-Slickgrid) but shouldn't that be the responsibility of the user to add such external library?

What I mean is that if you look at what I wrote in the README here, ngx-markdown is a nice to use library to preview the markdown but it's not a dependency, you could use whichever other library to see the preview result.

If I use the code that is written in the README here for the "Preview Button", I explicitely say is that it all depend on whichever library you use for parsing/compiling and if you use ngx-mardown (like I'm suggesting) then the code is this

import { MarkdownService } from 'ngx-markdown';

export class TestComponent implements OnInit {
  constructor(private markdownService: MarkdownService) {}

  ngOnInit() {
    this.editorOptions = {
      parser: (val) => this.markdownService.compile(val.trim())
    };
  }
}

then in your case, if you want to add DOMPurify then just do this.

this.editorOptions = {  
  parser: (val: string) => {
    const sanitizedText = DOMPurify.sanitize(val.trim());
    this.markdownService.compile(sanitizedText);
  }
};

So again I don't think it should be the responsibility of this lib to add DOMPurify with a strict dependency. Security is of course important but I think it should be handled by the user of the lib when adding whichever Markdown external library and setting up the parser callback. What I could do is to add a comment in the README to mention DOMPurify in the parser

I could be wrong in my assumptions since I haven't touch the code of this lib in like 2 years, but I think that just the parser setup with DOMPurify would be enough!? You can correct me if I'm wrong

[snoopysecurity]

You are right, I didn't notice the readme (https://github.com/ghiscoding/angular-markdown-editor#nice-to-have-dependencies) when i initially opened this and contacted you.

hmmm i am not sure about the DOMPurify usage either. Maybe its just worth mentioning DOMPurify as a nice to have dependency and leave it at that. anyone wanting to know more can reopen this or open a new issue.

[ghiscoding]

Maybe give it a try first and see if that works for you, then I'll update the readme

[snoopysecurity]

So i tested this out, using DOMPurify on the parser callback does indeed remove the malicious HTML when clicking on the Preview blue button but i can't seem to clean the output shown in the green Preview pane

[ghiscoding]

Well that text seems to be coming straight from the form here, just sanitize it there then.

[snoopysecurity]

Cool! works