elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.67k stars 8.23k forks source link

OWASP ZAP finding: Cross Site Scripting (Reflected in JSON Response) #101009

Closed pburkholder closed 2 years ago

pburkholder commented 3 years ago

Kibana version: 7.9.3

Elasticsearch version:

Server OS version: Ubuntu 18.04

Browser version: Firefox 88.0.01

Browser OS version: MacOS 10.

Original install method (e.g. download page, yum, from source, etc.): boshrelease

Describe the bug:

We're required to scan our web applications using OWASP ZAP for our compliance reporting, and since our update to 7.9.X a few months ago we have this finding for "Cross Site Scripting (Reflected)" (XSS Reflected)

It occurs when you make a POST https://(server)/api/core/capabilities and change the JSON request for "applications" to include a <script> element in the list, like this:

{"applications":["error","kibana","dev_tools","observability-overview","short_url_redirect","home","management","visualize","ingestManager","appSearch","workplaceSearch","apm","uptime","canvas","discover","dashboards","lens","<script>alert(1);</script>","maps","securitySolution","securitySolution:overview","securitySolution:detections","securitySolution:hosts","securitySolution:network","securitySolution:timelines","securitySolution:case","securitySolution:administration","siem","authentication","timelion"]}

The response then reflects the munged input:

{"navLinks":{"error":true,"kibana":true,"dev_tools":false,"observability-overview":true,"short_url_redirect":true,"home":true,"management":true,"visualize":true,"ingestManager":false,"appSearch":false,"workplaceSearch":false,"apm":false,"uptime":false,"canvas":true,"discover":true,"dashboards":true,"lens":true,"<script>alert(1);</script>":true,"maps":true,"securitySolution":false....

The malicious user input script>alert(1);</script> seems to be properly enclosed in quotes, and there's no evidence that the returned result is interpreted as Javascript, but it seems that the POST to api/core/capabilities should be filtered for potentially malicious user input.

Sorry I've not yet had a chance to test this against the latest Kibana. I'm up against reporting deadlines or I'd have done so.

Steps to reproduce:

  1. POST to api/core/capabilities a body of {"applications":["error","<script>alert(1);</script>"]}

Expected behavior:

Screenshots (if relevant): image

Any additional context:

elasticmachine commented 3 years ago

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine commented 3 years ago

Pinging @elastic/kibana-security (Team:Security)

elasticmachine commented 3 years ago

Pinging @elastic/kibana-core (Team:Core)

legrego commented 3 years ago

@pburkholder agree this is not exploitable within Kibana. In the future, please send all security reports to security@elastic.co so that we can remediate and responsibly disclose issues if they are in fact vulnerabilities.

I've assigned this to the core team for remediation

pburkholder commented 3 years ago

Just checking on whether this is still open. Thanks, Peter

pburkholder commented 3 years ago

Just checking on whether this is still open. Thanks, Peter

pburkholder commented 3 years ago

Whoops. Looks like I missed a month checking in on this. Any updates?

legrego commented 3 years ago

@elastic/kibana-core what do you think about restricting the input here to match the validations we put in place for feature ids?

https://github.com/elastic/kibana/blob/b6c982c3b073c215aa5bfa09887048ca549e2147/x-pack/plugins/features/server/feature_schema.ts#L23

pgayvallet commented 3 years ago

what do you think about restricting the input here to match the validations we put in place for feature ids

Overall, I don't have any objection to restrict the input of the /api/core/capabilities endpoint.

However, this applications array in the payload is the list of all registered apps' ids on the client-side.

Looking at the current list

['error', 'status', 'kibana', 'dev_tools', 'short_url_redirect', 'r', 'home', 'management', 'space_selector', 'security_access_agreement', 'security_capture_url', 'security_login', 'security_logout', 'security_logged_out', 'security_overwritten_session', 'security_account', 'canvas', 'graph', 'integrations', 'fleet', 'ingestManager', 'kibanaOverview', 'dashboards', 'maps', 'visualize', 'lens', 'observability-overview', 'discover', 'osquery', 'ml', 'uptime', 'securitySolution', 'siem', 'logs', 'metrics', 'infra', 'monitoring', 'enterpriseSearch', 'appSearch', 'workplaceSearch', 'apm', 'ux']

It seems the regexp would match. But if we do add regexp validation to the endpoint, we'll want to add the same validation during the client-side app registration (which would make sense, tbh I wasn't aware we're not performing any check here), else it would be possible to register an app that would cause the endpoint to reply with an error.

@legrego What's the urgency on this? Can it wait post 8, or should this be implemented in 7.16?

legrego commented 3 years ago

What's the urgency on this? Can it wait post 8, or should this be implemented in 7.16?

This doesn't pose an actual security risk to Kibana, so I don't personally think we need to re-prioritize other work to fit this into 7.16

pburkholder commented 2 years ago

Hi -- it's been about 3 months, and a busy December. Any prospect of picking this up again in January?

pgayvallet commented 2 years ago

Any prospect of picking this up again in January?

I just picked the issue to be discussed during our next prioritization meeting.

pburkholder commented 2 years ago

Great to hear this has been picked up -- did it get prioritized? If so, is there any public record I can point to on that? Thanks!

TinaHeiligers commented 2 years ago

@pburkholder the issue isn't prioritized yet but I've added it to our next meeting.

pburkholder commented 2 years ago

Hi @TinaHeiligers -- I hope you had a good month. I have federal compliance obligations to inquire on progress here. Meanwhile, here's a calming gif for your day:

calming waves

pgayvallet commented 2 years ago

@pburkholder thanks for the gif, it totally did inspire me!

Proof being, I'm glad to announce that I just opened a PR to address this issue: https://github.com/elastic/kibana/pull/129564

This should be fixed shortly, and will be available on 8.2.0+, and backported to 7.17.3