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

XSS in custom (root) certificate #1964

Closed
binaryfigments opened this Issue Nov 28, 2017 · 13 comments

Comments

Projects
None yet
4 participants
@binaryfigments

binaryfigments commented Nov 28, 2017

I do not think the impact will be big, it only works with crafted certificates logged in as root.

It works on:
OPNsense 17.1.1-amd64
FreeBSD 11.0-RELEASE-p7
OpenSSL 1.0.2k 26 Jan 2017

And a just complete upgraded firewall installation.

URLs:
https://firewall/system_camanager.php
https://firewall/system_certmanager.php

While adding a custom certificate authority, you can trigger an XSS in the admin panel.

First, I went to Authorities and added the CA. No problems here.
opnsense0

Then I click further to certificates. (for adding a certificate to that fake root.
Here is the first pop-up.

opnsense1

Clicking these away this is the result:

opnsense2

Adding a certificate over there:
opnsense3

Here are the CA and certificate files I used.
fakeca.zip

@AdSchellevis

This comment has been minimized.

Show comment
Hide comment
@AdSchellevis

AdSchellevis Nov 28, 2017

Member

@binaryfigments thanks for reporting, we will fix this before the next release.

Member

AdSchellevis commented Nov 28, 2017

@binaryfigments thanks for reporting, we will fix this before the next release.

@AdSchellevis

This comment has been minimized.

Show comment
Hide comment
@AdSchellevis

AdSchellevis Nov 28, 2017

Member

this 1219f2f should do the trick. can you test on your end to be sure?
opnsense-patch 1219f2f

Member

AdSchellevis commented Nov 28, 2017

this 1219f2f should do the trick. can you test on your end to be sure?
opnsense-patch 1219f2f

@binaryfigments

This comment has been minimized.

Show comment
Hide comment
@binaryfigments

binaryfigments Nov 28, 2017

Sure! I'll give it a try.

binaryfigments commented Nov 28, 2017

Sure! I'll give it a try.

@fabianfrz

This comment has been minimized.

Show comment
Hide comment
@fabianfrz

fabianfrz Nov 29, 2017

Member

@AdSchellevis maybe there should be a separate channel for those bugs like an email address with a public GPG key to encrypt the mails to make sure there will not be any exploits in the wild using already fixed bugs before the next release. For example this (stored XSS) can be used for privilege escalation.

CC @fichtner

Member

fabianfrz commented Nov 29, 2017

@AdSchellevis maybe there should be a separate channel for those bugs like an email address with a public GPG key to encrypt the mails to make sure there will not be any exploits in the wild using already fixed bugs before the next release. For example this (stored XSS) can be used for privilege escalation.

CC @fichtner

@fabianfrz

This comment has been minimized.

Show comment
Hide comment
@fabianfrz

fabianfrz Nov 29, 2017

Member

@AdSchellevis not completely fixed,

found also an XSS vulnerability in certificate details view:
screenshot_20171129_195915

Member

fabianfrz commented Nov 29, 2017

@AdSchellevis not completely fixed,

found also an XSS vulnerability in certificate details view:
screenshot_20171129_195915

@AdSchellevis

This comment has been minimized.

Show comment
Hide comment
@AdSchellevis

AdSchellevis Nov 29, 2017

Member

@fabianfrz it's quite unlikely that this will surface in the wild, but for sensitive information there are email addresses in place (project) that people can (and do) use.

I'm not entirely sure what the issue is in the certificate details view, your screenshots doesn't appear to have any script code in it.

Member

AdSchellevis commented Nov 29, 2017

@fabianfrz it's quite unlikely that this will surface in the wild, but for sensitive information there are email addresses in place (project) that people can (and do) use.

I'm not entirely sure what the issue is in the certificate details view, your screenshots doesn't appear to have any script code in it.

@fichtner

This comment has been minimized.

Show comment
Hide comment
@fichtner

fichtner Nov 30, 2017

Member

Either way, if people wish to disclose issues via GitHub they should be aware of their potential to be exploitable. Security issues can be reported via e-mail for coordinated fixes. The researcher / reporter is in charge of the course taken here.

Member

fichtner commented Nov 30, 2017

Either way, if people wish to disclose issues via GitHub they should be aware of their potential to be exploitable. Security issues can be reported via e-mail for coordinated fixes. The researcher / reporter is in charge of the course taken here.

@fabianfrz

This comment has been minimized.

Show comment
Hide comment
@fabianfrz

fabianfrz Nov 30, 2017

Member

@AdSchellevis it shows as empty because it is being executed with the provided certificate.

Member

fabianfrz commented Nov 30, 2017

@AdSchellevis it shows as empty because it is being executed with the provided certificate.

@AdSchellevis

This comment has been minimized.

Show comment
Hide comment
@AdSchellevis

AdSchellevis Nov 30, 2017

Member

check, fixed in 66c5335

Member

AdSchellevis commented Nov 30, 2017

check, fixed in 66c5335

@fichtner

This comment has been minimized.

Show comment
Hide comment
@fichtner

fichtner Nov 30, 2017

Member

To be completely frank, I will push the fix even untested and breaking the page just to fix the XSS ;)

Member

fichtner commented Nov 30, 2017

To be completely frank, I will push the fix even untested and breaking the page just to fix the XSS ;)

fichtner added a commit that referenced this issue Nov 30, 2017

system_certmanager, xss with crafted cert
PR: #1964

(cherry picked from commit 1219f2f)
(cherry picked from commit 66c5335)
@fichtner

This comment has been minimized.

Show comment
Hide comment
@fichtner

fichtner Nov 30, 2017

Member

merged to stable, happy about "ok" from the reporter despite ticket close :)

Member

fichtner commented Nov 30, 2017

merged to stable, happy about "ok" from the reporter despite ticket close :)

@fichtner fichtner closed this Nov 30, 2017

@fabianfrz

This comment has been minimized.

Show comment
Hide comment
@fabianfrz

fabianfrz Nov 30, 2017

Member

@AdSchellevis your change broke the output:
screenshot_20171130_164320

As you can see on the output, it strips tags. I provide a PR.

Member

fabianfrz commented Nov 30, 2017

@AdSchellevis your change broke the output:
screenshot_20171130_164320

As you can see on the output, it strips tags. I provide a PR.

@fichtner

This comment has been minimized.

Show comment
Hide comment
@fichtner

fichtner Dec 15, 2017

Member

@binaryfigments I just read your blog post and realised that another "thank you" is in order: thank you <3

For reference: https://binaryfigments.com/2017/12/11/dont-trust-all-ssl-tls-certificates/

Member

fichtner commented Dec 15, 2017

@binaryfigments I just read your blog post and realised that another "thank you" is in order: thank you <3

For reference: https://binaryfigments.com/2017/12/11/dont-trust-all-ssl-tls-certificates/

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