uiwjs / react-markdown-preview

React component preview markdown text in web browser. The minimal amount of CSS to replicate the GitHub Markdown style. Support dark-mode/night mode.
https://uiwjs.github.io/react-markdown-preview
MIT License
277 stars 49 forks source link

Sanitize HTML content #205

Closed tnhu closed 1 year ago

tnhu commented 2 years ago

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?

jaywcjlove commented 2 years ago

@tnhu

jaywcjlove commented 2 years ago

@tnhu Using rehype-sanitize is also ignorable.

tnhu commented 2 years ago

@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 commented 2 years ago

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

jaywcjlove commented 2 years ago

@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 commented 2 years ago

@tnhu Upgrade v4.1.2

tnhu commented 2 years ago

@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.
jaywcjlove commented 2 years ago

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.