Prevent cross-site image requests from leaking contents of certain fields due to regex search
Categories
(Bugzilla :: Reporting/Charting, defect)
Tracking
()
People
(Reporter: hofusec, Assigned: dylan)
References
Details
(Keywords: sec-critical, wsec-csrf, wsec-disclosure, Whiteboard: [reporter-external] [web-bounty-form] [verif?])
Attachments
(4 files, 15 obsolete files)
37.02 KB,
image/png
|
Details | |
4.80 KB,
patch
|
dkl
:
review+
|
Details | Diff | Splinter Review |
3.60 KB,
patch
|
jfearn
:
review+
|
Details | Diff | Splinter Review |
3.54 KB,
patch
|
Details | Diff | Splinter Review |
Via the image generation in report.cgi it is possible for a third-party website to extract confidential bug information if a logged-in user with access to a confidential bug visits the website. For example, a third-party website can access the summary of a security-sensitive bug report. The problem exists because via report.cgi complex queries to the bugzilla database can be done and if there is a result, an image will be returned, but if the query has no result no image is returned. Because of this a third-party website can extract information by detecting if certain images can be loaded or not. POC: 1.) download attached poc.html 2.) login to bugzilla 3.) open poc.html and enter a bug id of a confidential bug and the url of the bugzilla installation (I have tested the poc with URL: https://bugzilla.mozilla.org and Bug ID: 1395143) 4.) click on “Get Summary” After a while, most of the summary is displayed at the html file.
Reporter | ||
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Holger: thank you for your report, the PoC is very creative and really helps makes this more visible and easy to understand and test. That said, I did test this on multiple BMO accounts (1) with access to bug 1395143 and (2) without access to the bug. In my testing with an account that has access to the bug, it's not surprising that it was able to enumerate the bug summary, but that account already has access to that bug. In the test case without access to the bug, it just keeps attemping, but recognizes the char set as ".........." and continues to try without enumerating any content. Again, bang up job on the PoC presentation, but I wasn't able to replicate the behavior where a BMO user without access to the private bug could enumerate the bug summary. Please let me know if you think I'm doing this incorrectly and we can give it a go again.
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
On second review, I misread the original report, the issue you are stating is that a 3rd party site can access information from a user with access to the bug. I'm re-reviewing this now given that context.
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Ok, I've confirmed the behavior as described. I'll raise with the service owner now. Again, thanks for the report!
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Comment 6•6 years ago
|
||
So, to try and simplify the PoC a little more for easier digestion... Confirming first char X: https://bugzilla.mozilla.org/report.cgi?bug_id=1395143&bug_id_type=anyexact&short_desc=^x&short_desc_type=regexp&x_axis_field=short_desc&format=bar&ctype=png&action=plot&width=60&height=35 Confirming first char is NOT Y: https://bugzilla.mozilla.org/report.cgi?bug_id=1395143&bug_id_type=anyexact&short_desc=^y&short_desc_type=regexp&x_axis_field=short_desc&format=bar&ctype=png&action=plot&width=60&height=35 The issue being that a remote site can produce these URLs to leverage the users BMO session to generate them. My initial thoughts here is that the browser should prevent this from happening.
Assignee | ||
Comment 7•6 years ago
|
||
This is a bug in upstream bugzilla as well. I'm cc'ing jfearn for bugzilla.redhat.com on this so we can coordinate a fix. I will also need to push out new bugzilla 5 and bugzilla 4.4 releases.
Updated•6 years ago
|
Assignee | ||
Comment 8•6 years ago
|
||
We have one (load-balancer-based) mitigation deployed to bugzilla.mozilla.org. I'm working on a more general fix for the code and we'll be doing a security advisory for all bugzillas.
Updated•6 years ago
|
Assignee | ||
Comment 10•6 years ago
|
||
First patch for bmo. Also testing patches for 4.2, 4.4, 5.0 and master.
Assignee | ||
Comment 11•6 years ago
|
||
Man I hate patches. That was the wrong one. here is the right one.
Assignee | ||
Comment 12•6 years ago
|
||
Assignee | ||
Comment 13•6 years ago
|
||
This is only different in that the regex for adding a / doesn't chop off the last not-/ character. An edge case which can't happen in BMO.
Assignee | ||
Updated•6 years ago
|
Comment 14•6 years ago
|
||
:emorley, in response to your problems with bug 1433493: In theory, we could lock the Referrer patches down to only domains that we trust to not to run arbitrary JavaScript. Without a really strong Content Security Policy (currently not in place), I wouldn't trust any Etherpad codebase to not be vulnerable to XSS attacks, which could then be used against BMO. The codebase has a long history of XSS problems, which is why it was moved off of mozilla.org in the first place. :\
Updated•6 years ago
|
Assignee | ||
Comment 15•6 years ago
|
||
The regex wasn't correct for cases where the referer policy is 'origin'.
Assignee | ||
Comment 16•6 years ago
|
||
(In reply to April King [:April] from comment #14) > :emorley, in response to your problems with bug 1433493: > > In theory, we could lock the Referrer patches down to only domains that we > trust to not to run arbitrary JavaScript. Without a really strong Content > Security Policy (currently not in place), I wouldn't trust any Etherpad > codebase to not be vulnerable to XSS attacks, which could then be used > against BMO. The codebase has a long history of XSS problems, which is why > it was moved off of mozilla.org in the first place. :\ Note that we're only blocking non-html in the final patch. The mitigation is dumber than the final fix. Linking to images can't work.
Assignee | ||
Comment 17•6 years ago
|
||
Assignee | ||
Comment 18•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 19•6 years ago
|
||
Here is the patch to 5.0, which is somewhere between the bmo patch and the 4.4 patch. It will probably cleanly apply to master as well.
Comment 20•6 years ago
|
||
Comment on attachment 8946436 [details] [diff] [review] 1433400_3.1_bmo.patch Review of attachment 8946436 [details] [diff] [review]: ----------------------------------------------------------------- ::: Bugzilla/CGI.pm @@ +444,5 @@ > } > > + if (Bugzilla->usage_mode == USAGE_MODE_BROWSER) { > + > + # Safe content types are ones that arn't images. aren't @@ +450,5 @@ > + my $content_type = $headers{'-type'} // $headers{'-content_type'} // 'text/html'; > + my $is_safe_content_type = ( $content_type eq 'text/html' || $content_type eq 'text/plain' ); > + > + # Safe referers are ones that begin with the urlbase. > + my $referer = $self->referer; Isn't the referer easily spoofable?
Comment 21•6 years ago
|
||
It is, but not from within a web browser. The person with privileged access would have to be coerced into spoofing their referrer, and there's no easy way to make that happen.
Comment 22•6 years ago
|
||
Comment on attachment 8946436 [details] [diff] [review] 1433400_3.1_bmo.patch Review of attachment 8946436 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to April King [:April] from comment #21) > It is, but not from within a web browser. > > The person with privileged access would have to be coerced into spoofing > their referrer, and there's no easy way to make that happen. Ah correct. I was able to do so but had to install a simple web extension. The victim would have to intentionally do so. With that, this looks good and fixes the provided exploit for me. r=dkl
Comment 23•6 years ago
|
||
Comment on attachment 8946439 [details] [diff] [review] 1433400_3_bugzilla_4.4.patch Review of attachment 8946439 [details] [diff] [review]: ----------------------------------------------------------------- Aside from the unrelated white space changes this looks good :)
Assignee | ||
Comment 24•6 years ago
|
||
very good. dkl: let's sync up this week about doing the security release sometime next week.
Comment 25•6 years ago
|
||
Comment on attachment 8946439 [details] [diff] [review] 1433400_3_bugzilla_4.4.patch Review of attachment 8946439 [details] [diff] [review]: ----------------------------------------------------------------- On furtehr testing this patch breaks searches in Firefox, because the content type doesn't match. content_type: multipart/x-mixed-replace;boundary="------- =_lh0NRAkY1DxpHkn5K"
Assignee | ||
Comment 26•6 years ago
|
||
This one allows multipart responses and uses a regex that is faster (between 10 and 40 operations instead of 60-200 operations) However I think have found another problem -- this will block attachments when the attachment base is not the url base. I'll have to do a few more special things for attachments. I will carry forward the r+, but dkl can look over this version. The next one will just have an additional option to $cgi->header(): allow_unsafe_referer.
Assignee | ||
Comment 27•6 years ago
|
||
Comment 28•6 years ago
|
||
Comment on attachment 8948658 [details] [diff] [review] 1433400_5_bmo.patch Review of attachment 8948658 [details] [diff] [review]: ----------------------------------------------------------------- r=dkl
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 29•6 years ago
|
||
Assignee | ||
Comment 30•6 years ago
|
||
Assignee | ||
Comment 31•6 years ago
|
||
Assignee | ||
Comment 32•6 years ago
|
||
Okay. Let's start with 4.4 and if there are no issues with this, I'll do 5.0 and master (and 4.2 for good measure)
Assignee | ||
Comment 33•6 years ago
|
||
Bah, one more revision, Let's consider the following content types "safe": text/html application/rdf+xml application/atom+xml application/xml application/xml-dtd application/x-javascript application/json text/csv text/calendar text/plain This should take care of the situation you noticed on BRC, right?
Assignee | ||
Comment 34•6 years ago
|
||
Assignee | ||
Comment 35•6 years ago
|
||
Comment 36•6 years ago
|
||
Comment on attachment 8950462 [details] [diff] [review] 1433400_7_bugzilla-4.4.patch Review of attachment 8950462 [details] [diff] [review]: ----------------------------------------------------------------- state is too new, if you s/state/my/ it works fine.
Comment 37•6 years ago
|
||
(In reply to Dylan Hardison [:dylan] (he/him) from comment #33) > Bah, one more revision, Let's consider the following content types "safe": > > text/html > application/rdf+xml > application/atom+xml > application/xml > application/xml-dtd > application/x-javascript > application/json > text/csv > text/calendar > text/plain > > This should take care of the situation you noticed on BRC, right? Yeah I think this is correct, I'm not absolutely sure you can't tunnel queries through any of those formats though.
Assignee | ||
Comment 38•6 years ago
|
||
The only dangerous one would be text/xml, if we had something producing SVGs. otherwise the onload/onerror communication channel fails to work. So that's good. per our irc chat, I'll revise the 4.4 patch to avoid using the 'state' keyword since the files back in 4.4-land were not yet using the more-than-a-decade-old perl 5.10 syntax... :(
Assignee | ||
Comment 39•6 years ago
|
||
Without 'state'
Comment 40•6 years ago
|
||
Comment on attachment 8950824 [details] [diff] [review] 1433400_7_bugzilla-4.4.patch Review of attachment 8950824 [details] [diff] [review]: ----------------------------------------------------------------- Works for me!
Comment 42•6 years ago
|
||
Do we know when the upstream patches and CVE will be going live?
Assignee | ||
Comment 43•6 years ago
|
||
Assignee | ||
Comment 44•6 years ago
|
||
dkl has started the release process
Updated•6 years ago
|
Comment 46•6 years ago
|
||
Opening up as this is now public.
Comment 47•6 years ago
|
||
This has been committed to all related repos.
Assignee | ||
Comment 48•6 years ago
|
||
Thanks for handling the release dkl! Also thanks to both dkl and jfearn for reviewing the half-dozen revisions to this fix.
![]() |
||
Comment 49•6 years ago
|
||
Comment on attachment 8951341 [details] [diff] [review] 1433400_7_bugzilla-5.0.patch >--- a/Bugzilla/CGI.pm >+# responding to text/plain or text/html is safe This is confusing. This comment gives the feeling that those are the only two safe content types. You have to read the regexp to discover that there is more than that. >+sub _prevent_unsafe_response { >+ my ($self, $headers) = @_; >+ my $safe_content_type_re = qr{ >+ ^ (*COMMIT) # COMMIT makes the regex faster >+ # by preventing back-tracking. see also perldoc pelre. >+ # application/x-javascript, xml, atom+xml, rdf+xml, xml-dtd, and json >+ (?: application/ (?: x(?: -javascript | ml (?: -dtd )? ) >+ | (?: atom | rdf) \+ xml >+ | json ) >+ # text/csv, text/calendar, text/plain, and text/html >+ | text/ (?: c (?: alendar | sv ) Wouldn't it be more readable to write calendar | csv instead of this ugly c (?: alendar | sv ) ? >+ | plain >+ | html ) >+ # used for HTTP push responses >+ | multipart/x-mixed-replace) >+ }sx; >+ my $safe_referer_re = do { >+ # Note that urlbase must end with a /. >+ # It almost certainly does, but let's be extra careful. >+ my $urlbase = correct_urlbase(); >+ $urlbase =~ s{/$}{}; >+ qr{ >+ # Begins with literal urlbase >+ ^ (*COMMIT) >+ \Q$urlbase\E >+ # followed by a slash or end of string >+ (?: / >+ | $ ) >+ }sx >+ }; >+ >+ return if $ALLOW_UNSAFE_RESPONSE; What's the point of returning only now if $ALLOW_UNSAFE_RESPONSE is true instead of doing it immediately in this method, before anything else (especially calling correct_urlbase())? >+ if (!$is_safe_referer && !$is_safe_content_type) { >+ print $self->SUPER::header(-type => 'text/html', -status => '403 Forbidden'); >+ if ($content_type ne 'text/html') { This last check is useless. You already know that $content_type is not 'text/html', else $is_safe_content_type would be true and you wouldn't enter this block at all. Could the PoC be made public so that other security experts have the possibility to see how it works, please?
Comment 50•6 years ago
|
||
Holger, how would you like to be credited on our hall of fame? If you have a site you would like us to link to, we can do that as well. Thanks for your submission and for participating in our bug bounty program!
Comment 51•5 years ago
|
||
Hello,
Looks like this fix is prevent to update Bugzilla from version 4.4.12 to 4.4.13. Problem with: ^ (*COMMIT)
patch -p1 < bugzilla-4.4.12-to-4.4.13-nodocs.diff
patching file Bugzilla/CGI.pm
patching file Bugzilla/Constants.pm
patching file Bugzilla/DB/Sqlite.pm
patching file attachment.cgi
patching file taskgraph.json
patching file template/en/default/pages/release-notes.html.tmpl
./checksetup.pl
- This is Bugzilla 4.4.13 on perl 5.8.8
- Running on Linux 2.6.18-xenU-ec2-v1.0 #2 SMP Tue Feb 19 10:51:53 EST 2008
Checking perl modules...
Checking for CGI.pm (v3.51) ok: found v4.21
Checking for Digest-SHA (any) ok: found v5.95
Checking for TimeDate (v2.23) ok: found v2.24
Checking for DateTime (v0.28) ok: found v1.20
Checking for DateTime-TimeZone (v0.71) ok: found v1.93
Checking for DBI (v1.54) ok: found v1.634
Checking for Template-Toolkit (v2.22) ok: found v2.22
Checking for Email-Send (v2.04) ok: found v2.198
Checking for Email-MIME (v1.904) ok: found v1.936
Checking for URI (v1.37) ok: found v1.69
Checking for List-MoreUtils (v0.32) ok: found v0.413
Checking for Math-Random-ISAAC (v1.0.1) ok: found v1.004
Checking available perl DBD modules...
Checking for DBD-Pg (v2.7.0) not found
Checking for DBD-mysql (v4.001) ok: found v4.032
Checking for DBD-SQLite (v1.29) not found
Checking for DBD-Oracle (v1.19) not found
The following Perl modules are optional:
Checking for GD (v1.20) ok: found v2.45
Checking for Chart (v2.1.0) ok: found v2.4.1
Checking for Template-GD (any) ok: found v1.56
Checking for GDTextUtil (any) ok: found v0.86
Checking for GDGraph (any) ok: found v1.44
Checking for MIME-tools (v5.406) ok: found v5.427
Checking for libwww-perl (any) ok: found v5.834
Checking for XML-Twig (any) not found
Checking for PatchReader (v0.9.6) ok: found v0.9.6
Checking for perl-ldap (any) ok: found v0.39
Checking for Authen-SASL (any) ok: found v2.10
Checking for Net-SMTP-SSL (v1.01) ok: found v1.01
Checking for RadiusPerl (any) not found
Checking for SOAP-Lite (v0.712) ok: found v1.11
Checking for XMLRPC-Lite (v0.712) not found
Checking for JSON-RPC (any) not found
Checking for JSON-XS (v2.0) not found
Checking for Test-Taint (any) ok: found v1.06
Checking for HTML-Parser (v3.40) ok: found v3.71
Checking for HTML-Scrubber (any) ok: found v0.08
Checking for Encode (v2.21) ok: found v2.78
Checking for Encode-Detect (any) not found
Checking for Email-Reply (any) ok: found v1.202
Checking for HTML-FormatText-WithLinks (v0.13) not found
Checking for TheSchwartz (v1.07) not found
Checking for Daemon-Generic (any) not found
Checking for File-Slurp (v9999.13) not found
Checking for mod_perl (v1.999022) ok: found v2.000005
Checking for Apache-SizeLimit (v0.96) not found
Checking for File-MimeInfo (any) not found
Checking for IO-stringy (any) ok: found v2.110
Checking for mod_headers (any) ok
Checking for mod_expires (any) ok
Checking for mod_env (any) ok
- OPTIONAL MODULES *
- Certain Perl modules are not required by Bugzilla, but by *
- installing the latest version you gain access to additional *
- features. *
-
*
- The optional modules you do not have installed are listed below, *
- with the name of the feature they enable. Below that table are the *
- commands to install each module. *
-
MODULE NAME * ENABLES FEATURE(S) *
-
XML-Twig * Move Bugs Between Installations, Automatic Update Notifications *
-
RadiusPerl * RADIUS Authentication *
-
XMLRPC-Lite * XML-RPC Interface *
-
JSON-RPC * JSON-RPC Interface *
-
JSON-XS * Make JSON-RPC Faster *
-
Encode-Detect * Automatic charset detection for text attachments *
- HTML-FormatText-WithLinks * Inbound Email *
-
TheSchwartz * Mail Queueing *
-
Daemon-Generic * Mail Queueing *
-
File-Slurp * Mail Queueing *
-
Apache-SizeLimit * mod_perl *
-
File-MimeInfo * Sniff MIME type of attachments *
COMMANDS TO INSTALL OPTIONAL MODULES:
XML-Twig: /usr/bin/perl install-module.pl XML::Twig
RadiusPerl: /usr/bin/perl install-module.pl Authen::Radius
XMLRPC-Lite: /usr/bin/perl install-module.pl XMLRPC::Lite
JSON-RPC: /usr/bin/perl install-module.pl JSON::RPC
JSON-XS: /usr/bin/perl install-module.pl JSON::XS
Encode-Detect: /usr/bin/perl install-module.pl Encode::Detect
HTML-FormatText-WithLinks: /usr/bin/perl install-module.pl HTML::FormatText::WithLinks
TheSchwartz: /usr/bin/perl install-module.pl TheSchwartz
Daemon-Generic: /usr/bin/perl install-module.pl Daemon::Generic
File-Slurp: /usr/bin/perl install-module.pl File::Slurp
Apache-SizeLimit: /usr/bin/perl install-module.pl Apache2::SizeLimit
File-MimeInfo: /usr/bin/perl install-module.pl File::MimeInfo::Magic
To attempt an automatic install of every required and optional module
with one command, do:
/usr/bin/perl install-module.pl --all
Quantifier follows nothing in regex; marked by <-- HERE in m/
^ (* <-- HERE COMMIT) # COMMIT makes the regex faster
# by preventing back-tracking. see also perldoc pelre.
# application/x-javascript, xml, atom+xml, rdf+xml, xml-dtd, and json
(?: application/ (?: x(?: -javascript | ml (?: -dtd )? )
| (?: atom | rdf) + xml
| json )
# text/csv, text/calendar, text/plain, and text/html
| text/ (?: c (?: alendar | sv )
| plain
| html )
# used for HTTP push responses
| multipart/x-mixed-replace)
/ at Bugzilla/CGI.pm line 306, <DATA> line 275.
Compilation failed in require at Bugzilla.pm line 25, <DATA> line 275.
BEGIN failed--compilation aborted at Bugzilla.pm line 25, <DATA> line 275.
Compilation failed in require at ./checksetup.pl line 73, <DATA> line 275.
Description
•