Skip to content

Conversation

huntr-helper
Copy link

https://huntr.dev/users/d3v53c has fixed the Cross-site Scripting (XSS) vulnerability 🔨. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/zingchart-react/1/README.md

User Comments:

📊 Metadata *

zingchart-react is vulnerable to Cross-Site Scripting (XSS).

Bounty URL: https://www.huntr.dev/bounties/1-npm-zingchart-react

⚙️ Description *

Cross-Site Scripting (XSS) attacks are a type of injection, in which malicious scripts are injected into otherwise benign and trusted websites. XSS attacks occur when an attacker uses a web application to send malicious code, generally in the form of a browser side script, to a different end user. Flaws that allow these attacks to succeed are quite widespread and occur anywhere a web application uses input from a user within the output it generates without validating or encoding it.

💻 Technical Description *

Cross-Site Scripting (XSS) attacks are mitigated by sanitizing the user inputs before rendering, thereby preventing malicious execution.

🐛 Proof of Concept (PoC) *

Open https://github.com/zingchart/zingchart-react
Open link in about https://www.zingchart.com/docs/integrations/react
Open in Sandbox https://codesandbox.io/s/zingchart-react-wrapper-example-dxfc9?from-embed
Insert the xss payload in any of the values field in series in Simple.jsx. EX:

values: [4, '><img src=x onerror=alert(1)>', 3, 4, 5, 3, 5, 4, 11]

XSS payload will get executed.

🔥 Proof of Fix (PoF) *

Before:

poc-before

After:

poc-after

👍 User Acceptance Testing (UAT)

poc-test

After the fix, functionality is unaffected.

d3v53c and others added 3 commits November 29, 2020 04:38
Cross-Site Scripting (XSS) : issue fix by sanitizing strings before rendering
Copy link
Contributor

@mercurio mercurio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get this to replicate, perhaps there have been changes to the core library since this was posted that resolve this issue.

This also doesn't seem to be the right approach, I would think you'd want to sanitize user input, not what the programmer has entered as the configuration for a chart.

@JamieSlome
Copy link

@d3v53c - any thoughts on the above comment?

@d3v53c
Copy link

d3v53c commented Dec 7, 2021

I couldn't get this to replicate, perhaps there have been changes to the core library since this was posted that resolve this issue.

Let me take a closer look, probably the core library fixed it already.

This also doesn't seem to be the right approach, I would think you'd want to sanitize user input, not what the programmer has entered as the configuration for a chart.

User input should be sanitized, I concur, there's no argument in that. Also, insecure handling of such inputs should be sanitized too. If the issue is still persisting, I would argue that this is such a scenario where the core library is accepting an input (either developer config or user input, for all I know somebody could accept a user input and then pass it as a configuration), leading to a potential security breach. Anyway, please let me take a look on this issue and update here.

@JamieSlome
Copy link

@d3v53c - appreciate your contribution here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants