Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does html-react-parser strip out XSS? #94

Closed
dave-stevens-net opened this issue Mar 8, 2019 · 15 comments
Closed

Does html-react-parser strip out XSS? #94

dave-stevens-net opened this issue Mar 8, 2019 · 15 comments
Labels
question Further information is requested

Comments

@dave-stevens-net
Copy link

I'm wanting to use html-react-parser to sanitize and parse HTML from my CMS. Does it effectively sanitize the input from XSS attacks? https://stackoverflow.com/questions/29044518/safe-alternative-to-dangerouslysetinnerhtml#answer-48261046 claims that it does. If so, I think it would be great to document / advertise this somewhere in the README. Thanks for your work on this.

@remarkablemark
Copy link
Owner

Great question @dave-stevens-net!

Unfortunately it doesn't. The reason is because I chose to make this library flexible rather than strict.

Although there is the replace option, checking against all possible attacks may be too much. I recommend instead using an XSS sanitizer with dangerouslySetInnerHTML.

@remarkablemark remarkablemark added the question Further information is requested label Mar 8, 2019
@dave-stevens-net
Copy link
Author

Good to know. Thanks for the quick response.

@remarkablemark
Copy link
Owner

You're very welcome. If this answers your question @dave-stevens-net, can the issue be closed?

@remarkablemark
Copy link
Owner

@dave-stevens-net I may have misspoke earlier about this library not being XSS safe.

I originally thought this library wasn't XSS-safe because dangerouslySetInnerHTML was relied here.

However, it seems that I'm unable to reproduce any XSS vulnerabilities. See my fiddle, which is based off of this example.

Let me know if you have any luck in reproducing XSS attacks.

@remarkablemark remarkablemark reopened this Mar 9, 2019
@harveydf
Copy link

harveydf commented Mar 13, 2019

I managed to reproduce a simple XSS attack. There might be more.

Check my fiddle.

I found it in here https://www.in-secure.org/misc/xss/xss.html

@dave-stevens-net
Copy link
Author

I ended up coding a Sanitize component using the sanitize-html package dependency.

import React from 'react'
import sanitizeHtml from 'sanitize-html'

const Sanitize = ({ html }) => {
    const clean = sanitizeHtml(html, {
        allowedTags: sanitizeHtml.defaults.allowedTags.concat(['img', 'span']),
        allowedAttributes: {
           ...
        },
    })
    return (
        <span
            className="sanitized-html"
            dangerouslySetInnerHTML={{ __html: clean }}
        />
    )
}
export default Sanitize

Example usage:

<Sanitize html={data.wordpressPage.title} />

@remarkablemark
Copy link
Owner

@harveydf Great find! Thanks for creating and sharing the fiddle.

I'll update the README.md to note that this library isn't XSS safe.

@k1sul1
Copy link

k1sul1 commented Jul 9, 2019

I didn't want to use sanitize-html, because it's massive. I used dompurify instead, it's 10 times smaller, and doesn't remove CSS.

import parse, { domToReact } from 'html-react-parser'
import DOMPurify from 'dompurify'
import React from 'react'

// export function replaceNode() {}

export default function html(html, opts = {}) {
  return parse(DOMPurify.sanitize(html), {
    ...{
      replace: replaceNode,
    },
    ...opts,
  })
}

html('<iframe src=javascript:alert("xss")></iframe>')

@remarkablemark
Copy link
Owner

Thanks for sharing your approach using dompurify @k1sul1!

I created a Repl.it demo based on your example.

@xkcdstickfigure
Copy link

I managed to reproduce a simple XSS attack. There might be more.

Check my fiddle.

I found it in here https://www.in-secure.org/misc/xss/xss.html

Hey I know this is a pretty old comment but I just wanted to point out that this isn't actually an XSS issue since the JavaScript is running within the iframe. If you change the html to <iframe src=javascript:alert(location.href)></iframe>, you'll see that the URL it's running on is about:blank rather than the host page.

@alexgleason
Copy link

In the replace function, you can check domNode.name... so wouldn't it be inherently not possible to embed a script tag or iframe there if you just check if (['script', 'iframe'].includes(domNode.name)) return null ?

@remarkablemark
Copy link
Owner

@alexgleason there are many other ways to do XSS without <script> or <iframe>. For example:

<a onmouseover="alert()">xss</a>

Take a look at https://cheatsheetseries.owasp.org/cheatsheets/XSS_Filter_Evasion_Cheat_Sheet.html

@alexgleason
Copy link

Ahh... that makes sense.

What I'm really trying to figure out is if this library is any worse than dangerouslySetInnerHTML. Is there a new attack surface outside of what's already possible with dangerouslySetInnerHTML?

@remarkablemark
Copy link
Owner

@alexgleason you should treat this library the same as dangerouslySetInnerHTML if you didn't sanitize the HTML string

@alexgleason
Copy link

Thank you for clarifying. A friend of mine got burned by this one earlier this year, so now I am extra paranoid:

@graf does btrfly support pleroma <a href='\r\nd&#x61t&#x61:text/html,<scr&#x69pt></scr&#x69pt\" src=\"https://i.poastcdn.org/b2977f2d97f598d2ebd6dcf37afd9047b5da2b6dc95a7b2824fb111c906fb117.js\" hidden'></a>

Fortunately I can't reproduce the attack using this library. I just gave it a try.

They were using a custom HTML parser that was vulnerable. This library seems to use the browser's DOMParser when it's availble. Therefore, I conclude it's no less secure than using dangerouslySetInnerHTML directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants