apostrophecms / sanitize-html

Clean up user-submitted HTML, preserving whitelisted elements and whitelisted attributes on a per-element basis. Built on htmlparser2 for speed and tolerance
MIT License
3.68k stars 349 forks source link

Disallowed attributes are not getting sanitized properly #558

Closed dhiliphvenkatesan closed 1 year ago

dhiliphvenkatesan commented 1 year ago

To Reproduce

If i give html tag with improper syntax like this => <<imgimg src=1 onerror=alert('xss')>

Step by step instructions to reproduce the behavior:

  1. Go to https://codesandbox.io/s/kind-euler-jp6k9f?file=/src/components/HelloWorld.vue
  2. Click on Sanitize Html Button
  3. It will show unsanitized img tag with onerror attribute

Expected behavior

<img src=1>

Describe the bug

I am using this configuration: sanitizeHtml(this.sanitizedValue, { allowedTags: [ 'p', 'img', 'canvas' ], allowedAttributes: { '*': [ ['src'] ] } })

If i am giving the following string as an input => <<imgimg src=1 onerror=alert('xss')> Expected Output: <img src=1> Actual Output: <img src=1 onerror=alert('xss')>

Note: I have included canvas tag in the actual string but here it is getting executed so please refer my screenshot and codesandbox for actual string input and output

I expect the onerror attribute to be removed as i have not mentioned it in my configuration but it is not sanitizing the string

Details

Version of Node.js: Version 16.13.2

Server Operating System: Linux - ubuntu 20.04.03 64-bit

Screenshots sanitizehtml

boutell commented 1 year ago

The output is escaped, both in your codesandbox and when I add this code as a unit test. It's not returning markup, it's escaping the angle brackets as plaintext. So there's no issue here as far as I can tell. Certainly it's bad input, but the output is as reasonable as any result could be, given how unreasonable the input is.

But please let me know if I'm missing something and there's a way to exploit this.

boutell commented 1 year ago

(btw you don't want to pass a double array for attributes. I checked it with and without that though, same result.)