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

fix XSS vulnerabilities in dashboard links #11813

Merged
merged 4 commits into from May 8, 2018

Conversation

alexanderzobnin
Copy link
Contributor

This PR fixes XSS vulnerabilities in dashboard link components. This happens when you put html with XSS as a link title, like <img src='' onerror=alert("XSS") /> in
Panel -> General -> Drilldown / detail link -> Title or
Dashboard -> Links -> Tooltip

@1Jesper1
Copy link

1Jesper1 commented May 3, 2018

Hello, good to see a PR. I found this vulnerability when I typed XSS in the Drilldown / detail link in the title field. After filling in the title field the dashboard field was automatically filled with the same value (XSS). When adding the detail link and hovering over the information icon, the XSS was also executed.

@alexanderzobnin
Copy link
Contributor Author

@1Jesper1 so could you confirm this PR fixes it?

@1Jesper1
Copy link

1Jesper1 commented May 4, 2018

Yes, this PR fixes it.

Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

Code looks good and it fixes XSS problems with link titles and tooltips.

Would be nice with some XSS tests for the link_srv though.

I tried and added javascript:alert('XSS') to the url (for absolute url's) and clicking such a link executes the javascript. Don't know if we should create a separate issue for that or try to solve it in this. What do you think?

@alexanderzobnin
Copy link
Contributor Author

I think we can cover several XSS issues by this PR.

@marefr marefr merged commit 00454b3 into grafana:master May 8, 2018
@marefr marefr added this to the 5.2 milestone May 8, 2018
marefr added a commit that referenced this pull request May 8, 2018
ryantxu added a commit to NatelEnergy/grafana that referenced this pull request May 8, 2018
* grafana/master:
  Templating : return __empty__ value when all value return nothing to prevent elasticsearch syntaxe error (grafana#9701)
  http_server: All files in public/build have now a huge max-age (grafana#11536)
  fix: ldap unit test
  only error log when err is not nil
  rename alerting engine to service
  case-insensitive LDAP group comparison (grafana#9926)
  changelog: add notes about closing grafana#11813
  docs: updated changelog
  fix XSS vulnerabilities in dashboard links (grafana#11813)
  PR: ux changes to grafana#11528
  decrease length of auth_id column in user_auth table
  PR comments
  Make dashboard JSON editable
@1Jesper1
Copy link

Good to see this issue fixed! When will this be rolled out?

@marefr
Copy link
Member

marefr commented May 15, 2018

@1Jesper1 Currently targeted for the upcoming 5.2 release (no release date yet).

JanZerebecki added a commit to JanZerebecki/grafana that referenced this pull request Jun 13, 2018
Dropped the whitespace changes for public/test/specs/helpers.js while
manually applying the change.

Backport of grafana#11813
(cherry picked from commit 00454b3)
@noasand
Copy link

noasand commented Jan 28, 2019

The same vulnerability also occurs in the following places:

  1. Dashboard > Add Panel > Text Panel > Set Options -> Mode(html) & Content(<script>alert('XSS');</script>)
  2. Dashboard > Add Panel > Table Panel > Set Column Styles > Check Render value as link > set Url(javascript:alert('XSS'))
  3. Dashboard > Dashboard Settings > Set dashboard name(TEST<script>alert('XSS')</script> > Save dashboard and go back to dashboard home > Edit any panel > Add link, in General > Write the 'TEST' on the dashboard blank

And, these vulnerabilities are currently registered as CVE.
(CVE-2018-18623, CVE-2018-18624, CVE-2018-18625)

How can I get help to fix it?

@marefr
Copy link
Member

marefr commented Jan 28, 2019

@noasand for 1) will be fixed in next major release by #4117. 2) and 3) Please open a new issue. Thanks

@noasand
Copy link

noasand commented Jan 28, 2019 via email

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

Successfully merging this pull request may close these issues.

None yet

4 participants