react-markdown-preview icon indicating copy to clipboard operation
react-markdown-preview copied to clipboard

Sanitize HTML content

Open tnhu opened this issue 2 years ago • 8 comments

react-markdown-preview does not sanitize HTML content before rendering. Paste below code into https://uiwjs.github.io/react-markdown-preview and you'll see an alert showing up.

<div onmouseover="alert('alpha')">
  <a href="jAva script:alert('bravo')">delta</a>
  <img src="x" onerror="alert('charlie')">
  <iframe src="javascript:alert('delta')"></iframe>
  <math>
    <mi xlink:href="data:x,<script>alert('echo')</script>"></mi>
  </math>
</div>
<script>
require('child_process').spawn('echo', ['hack!']);
</script>

Maybe https://github.com/rehypejs/rehype-sanitize should be included?

tnhu avatar Oct 12 '22 22:10 tnhu

@tnhu

  • skipHtml (boolean, default: false) Ignore HTML in Markdown completely

jaywcjlove avatar Oct 13 '22 00:10 jaywcjlove

@tnhu Using rehype-sanitize is also ignorable.

jaywcjlove avatar Oct 13 '22 00:10 jaywcjlove

@jaywcjlove skipHtml does not work. The html content is still injected.

import ReactMarkdownPreview from '@uiw/react-markdown-preview'

const source = `<div onmouseover="alert('alpha')">
  <a href="jAva script:alert('bravo')">delta</a>
  <img src="x" onerror="alert('charlie')">
  <iframe src="javascript:alert('delta')"></iframe>
  <math>
    <mi xlink:href="data:x,<script>alert('echo')</script>"></mi>
  </math>
</div>
<script>
require('child_process').spawn('echo', ['hack!']);
</script>`

...

<ReactMarkdownPreview source={source} skipHtml />  

--> delta is shown in an alert

tnhu avatar Oct 17 '22 22:10 tnhu

You can try it here - https://codesandbox.io/s/floral-leaf-s0t7hm?file=/src/App.js

tnhu avatar Oct 17 '22 22:10 tnhu

@tnhu https://codesandbox.io/embed/uiwjs-react-markdown-preview-issues-205-kl1xdq?fontsize=14&hidenavigation=1&theme=dark

import MarkdownPreview from "@uiw/react-markdown-preview";

const source = `<div onmouseover="alert('alpha')">
<a href="jAva script:alert('bravo')">delta</a>
<img src="x" onerror1="alert('charlie')">
<iframe src="javascript:alert('delta')"></iframe>
<math>
  <mi xlink:href="data:x,<script>alert('echo')</script>"></mi>
</math>
</div>
<script>
require('child_process').spawn('echo', ['hack!']);
</script>`;

export default function App() {
  return (
    <div className="App">
      <MarkdownPreview
        source={source}
        skipHtml={false}
      />
    </div>
  );
}

jaywcjlove avatar Oct 18 '22 02:10 jaywcjlove

@tnhu Upgrade v4.1.2

jaywcjlove avatar Oct 18 '22 02:10 jaywcjlove

@jaywcjlove it works. Thank you very much! skipHtml={false} removed support for raw HTML, as a result, it prevents security issue like above.

Just to add a little more:

  1. Should it be skipHtml={true} to remove raw HTML instead of skipHtml={false}? skip something means ignore it. In this context, I think skip equals to false but actually removing support for it is confusing.
  2. Raw HTML should be ignored by default. Don't you think?
  3. When HTML is ignored (currently skipHtml={false}), all HTML tags in the markdown source are ignored. It's good for security purposes but it's harsh as we can't have some small nice things like <quote>, etc... It'd be best if somehow we can filter out the malicious ones, but still keep simple tags that we support.

tnhu avatar Oct 20 '22 03:10 tnhu

https://github.com/uiwjs/react-markdown-preview/blob/de60a8500860651b161980f83b672580420db7c4/src/index.tsx#L82-L89

@tnhu Parse html by default This is because I have many projects using this package, Keep the original features.

skipHtml={false} does not parse HTML by default, which is a feature of the dependent react-markdown package.

jaywcjlove avatar Oct 20 '22 05:10 jaywcjlove