[Bug] Dropdown component vulnerable to XSS attacks

[tvt-devteam]

Affected Projects React

Library Version: 3.22.0

Describe the bug

User inputted HTML tags are not sanitized by ReactiveSearch. They are rendered using dangerouslySetInnerHTML, and will get inserted into the DOM as such.

To Reproduce

Steps to reproduce the behavior:

1. Insert an HTML tag into ElasticSearch. We used <img src onerror("alert('Testing');">
2. Have a component that uses the shared Dropdown component search and load this data into UI
3. Javascript inside the onerror function gets executed

Expected behavior User inserted HTML tag should be either sanitized to not contain HTML tag characters, or just be rendered as a string.

Screenshots

BUG

1. User inserts a tag they want to be associated with their image. This get inserted into ElasticSearch.

2. We use MultiDropdownList component to render all tags found within ES.

3. The component fails to show the given tag label as a string, as it is rendered into DOM instead.

INTENTED

4. The tag label is rendered as a string

Desktop (please complete the following information):

• OS: MacOS
• Google Chrome
• Version 92.0.4515.131

Smartphone (please complete the following information):

Has not been tested on mobile

Additional context

Link to component that is (most likely) causing the issue

https://github.com/appbaseio/reactivesearch/blob/13a29bab1db06f9ccf5964c625f693bc106baba0/packages/web/src/components/shared/Dropdown.js#L228

NOTE: This has been only slightly tested by us and might not be the best or even correct solution, but it got us some positive results.

Proposed solution:

Change:

typeof item[labelField] === 'string' ? (0, _core.jsx)(
'span',
{
    dangerouslySetInnerHTML: {
        __html: item[labelField]
    },
})
: item[labelField],

into:

typeof item[labelField] === 'string' ? (0, _core.jsx)(
 'span',
{
  className: (0, _helper.getClassName)(_this2.props.innerClass, 'label') || null
},
item[labelField]) : item[labelField],

[tvt-devteam]

Bump - is there any info on this, or has this been looked at? How should we proceed with this problem?

[siddharthlatest]

@tvt-devteam We're supporting HTML rendering to allow a user control over how the UI gets rendered. Since the user is both in control of rendering / their Elasticsearch, I don't see how this is vulnerable to misuse.

Sanitizing the input here would have the side-effect of users not being able to use rich HTML, so this isn't an ideal solution imo.

[tvt-devteam]

Thanks for the answer @siddharthlatest

We looked into the component some more and indeed did find the renderItem prop and was able to fix this problem on our end!

We are still thinking that use of this component is a bit risky because of it defaulting to use DangerouslySetInnerHTML.

Maybe there could be a prop to enable/disable the use of this way of rendering, which would be disabled by default?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.