Closed Bug 1253673 (CVE-2016-5268) Opened 9 years ago Closed 9 years ago

bypass FireFox Secure Connection Failed prompt to whitelist any site (but doesn't work)

Categories

(Core :: Networking, defect)

Product:
Core
Component:
Networking
Version:
38 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox48 --- fixed

People

(Reporter: todayisnew, Assigned: Gijs)

References

Details

(Keywords: addon-compat, csectype-spoof, sec-low, Whiteboard: [adv-main48+])

Attachments

(3 files)

CustomMITMBypassMessages.png
20.29 KB, image/png
Details
Patch - about: changes + DOM tests
19.82 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Fix miscellaneous browser tests to deal with this change
11.69 KB, patch
jaws
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36 Steps to reproduce: FireFox has an internal about: url scheme about:downloads for example shows your downloads In the case of a mitm attack FireFox shows a "Secure Connection Failed" allowing the user to overide if they feel safe. The problem is that this url scheme can be modifed to let us set any custom content to trick the user into whitelisting the bad certificate. Also possible for xss, javascript injection or one of many other about: internal url calls. http://me6.com/mozilla/poc1.html <html> hi <script> window.addEventListener("load",function() { window.location= 'about:neterror?e=nssBadCert&u=anything1&f=anything2&d=<a%20id="cert_domain_link"%20title="Because of Pugs... FireFox Says This Site is super duper safe .com.">&s=anythingForImage</a>I know we have told you not to trust this cert but FireFox says it is safe click on the add exception below on the bottom of the page..'; }); </script> </html> Filterning is in place for about:downloads We will get "Error: Access to 'about:downloads' from script denied" http://me6.com/mozilla/poc2.html <html> hi2 <script> window.addEventListener("load",function() { window.location = "about:downloads"; }); </script> </html> I suggest FireFox implment the same Access Restrictions to many of the about: scripts used for about:downloads. There are multiple internal security methods exposed, that external websites should not have access to load, modify, inject content into. Wish you well on your side of the screen :) -Eric Actual results: Allowed to provide custom content in fields Expected results: Restricted the ability to set custom content in internal pages
(In reply to todayisnew from comment #0) > The problem is that this url scheme can be modifed to let us set any custom > content to trick the user into whitelisting the bad certificate. Also > possible for xss, javascript injection or one of many other about: internal > url calls. > > > http://me6.com/mozilla/poc1.html Do you have a PoC that actually shows what you describe (XSS, JS injection, or calling other internal about: URIs) ? As it is, the "add an exception" button on your PoC does not actually do anything, presumably because the right params were not passed... In order for spoofing here to be useful, it should be possible to convince the user to do something they wouldn't normally do; otherwise you could just recreate the HTML page and that way you could manipulate much more of the content. :-) > I suggest FireFox implment the same Access Restrictions to many of the > about: scripts used for about:downloads. > > There are multiple internal security methods exposed, that external websites > should not have access to load, modify, inject content into. Boris, do we need to allow content to link to about:neterror and about:certerror at all ? We can now make that decision orthogonally from whether we run it with content privileges... (bug 1150703) and using that flag would put an end to this avenue of spoofing / confusing users.
Flags: needinfo?(todayisnew)
Flags: needinfo?(bzbarsky)
> Boris, do we need to allow content to link to about:neterror and about:certerror at all ? I would think we don't; have you checked the history of why we do allow it?
Flags: needinfo?(bzbarsky)
(In reply to Boris Zbarsky [:bz] from comment #2) > > Boris, do we need to allow content to link to about:neterror and about:certerror at all ? > > I would think we don't; have you checked the history of why we do allow it? Before bug 1150703 to the choice was: 1) unlinkable but chrome privileged 2) linkable but not chrome privileged. You could not have both unlinkable and content privileges. So I imagine that back when we added SAFE_FOR_UNTRUSTED_CONTENT, to a certain degree that was a step up from the alternative, and we tagged as much as possible with it. Looks like the flag was added in bug 337746, so you probably have a better idea than me... ;-) It might be worth to just audit all our things and, unless there are very good reasons not to, add the flag to all of them - or tweak the flags so we have the inverse (MAKE_LINKABLE or something), make 'unlinkable' the default for all about: URIs, and make the MAKE_LINKABLE flag override that, and make that flag only work for SAFE_FOR_UNTRUSTED_CONTENT urls. That would have add-on compat issues etc., and would potentially require fixes to some of our tests... but it would be the 'safe by default' choice here. Of course, someone would have to do the work...
> Looks like the flag was added in bug 337746, so you probably have a better idea than me Well, "neterror" being considered "safe" predates that bug, right? It was in the old "about safe" set.... But yes, in general making things not linkable by default makes sense to me.
Hmm for a xss example we could chain with an internal about url scheme: about:SocialError Then use data url scheme base64 encoded to bypass the encoding to execute the code? about:neterror?e=nssBadCert&u=anything1&f=anything2&d=<a%20id="cert_domain_link"%20title="SocialError?directory=no&mode=tryAgain&url=data:text/html;base64,amF2YXNjcmlwdDphbGVydCgxMjMp&origin=.com.">&s=anythingForImage</a>
Flags: needinfo?(todayisnew)
FWIW, I am fairly convinced that this *could* in principle be used for spoofing, but it needs some more work to get the parameters right. The example link, for instance, does not pass a URL to use, and it misses an errorCode as well, leading to exceptions in the about:neterror/about:certerror code. On Nightly, it seems there's some code that tries to read failedChannel which obviously doesn't work when the page is not actually loaded as the result of an error. So I'm not even entirely sure that it can be made to work at all there, and/or which versions would be vulnerable if someone put in the work to attempt to construct a fully working testcase, which nobody seems to have done so far. IMO the best fix is the suggestions from comment 3 / comment 4. However, I don't know that I can justify putting in the work to pursue comment 3 / comment 4 right now due to our focus on e10s. A trivial fix is to add the existing unlinkable flags to about:neterror and about:certerror, but that feels like whack-a-mole. Another option would be being more deliberate within about:neterror/about:certerror about hiding all text and UI if the page is explicitly loaded without a failedChannel and other paraphernalia associated with an actual error, which might be useful as defense in depth anyway.
So far these are spoofs that don't follow through so calling it sec-low, but it would be more severe if you could actually get to the cert exception dialog or inject script.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: csectype-spoof, sec-low
Summary: bypass FireFox Secure Connection Failed prompt to whitelist any site allowing a MITM attack → bypass FireFox Secure Connection Failed prompt to whitelist any site (but doesn't work)
No worries, if it is not needed to allow external sites to access this url scheme, maybe limit it to FireFox browser access internally? If public sites are going to be allowed to continue to request and inject into the "about:neterror" scheme, i'll dig deeper and get a working poc. wish you and yours well on your side of the screen :) -Eric
(In reply to todayisnew from comment #8) > No worries, if it is not needed to allow external sites to access this url > scheme, maybe limit it to FireFox browser access internally? Yes, the problem is that doing this is harder than one might first think... I have a work in progress patch, but it looks like it might be a while before I get all the automated tests fixed...
(In reply to :Gijs Kruitbosch from comment #9) > (In reply to todayisnew from comment #8) > > No worries, if it is not needed to allow external sites to access this url > > scheme, maybe limit it to FireFox browser access internally? > > Yes, the problem is that doing this is harder than one might first think... > I have a work in progress patch, but it looks like it might be a while > before I get all the automated tests fixed... Right, this has everything I noticed locally fixed... let's see what try makes of it. remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1112ac3b1081
I kept about:logo linkable mostly because I think it should be tricky to do any damage that way, as it's just an image, and because there were quite some tests that seemed to rely on it and I am lazy. Otherwise I've kept all the things that use indexed_db the same because making them unlinkable depends on un-nesting about: URIs and I didn't want to tackle that in the same patch. I also don't know how snippets will deal with making about:home be unlinkable, but we can cross that bridge when we get there - this just keeps the status quo.
Attachment #8741401 - Flags: review?(bzbarsky)
These are all the other tests I had to fix for the last trypush to be as green as it is. Mostly browser-land, so Jared, picking on you - let me know if I need to find someone else.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #8741402 - Flags: review?(jaws)
(Jared, note that splinter has messed up its display of file copies as per usual... I copied the dummy.html file, and it thinks I copied one of the test files to itself, or something. When in doubt, refer to the diff.)
(In reply to :Gijs Kruitbosch from comment #12) > I kept about:logo linkable mostly because I think it should be tricky to do > any damage that way, as it's just an image, and because there were quite > some tests that seemed to rely on it and I am lazy. (FWIW, I'd be happy to fix it in a followup, but I felt this patch was big enough as it is.)
Comment on attachment 8741401 [details] [diff] [review] Patch - about: changes + DOM tests The entries with MAKE_LINKABLE should have comments explaining why they're linkable. For the ones that depend on idb, they should reference a bug number that depends on the idb/about-unnesting thing. MAKE_UNLINKABLE in the IDL is now meaningless, since no one checks it, no? Or do we still have code checking it somewhere? For compat, we should probably keep checking for it wherever we used to... r=me with those fixed.
Attachment #8741401 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #16) > MAKE_UNLINKABLE in the IDL is now meaningless, since no one checks it, no? Yes. Moreover, its meaning is subsumed into the URI_SAFE_FOR_UNTRUSTED_CONTENT flag - just specifying that now implies UNLINKABLE (unless you specify the new LINKABLE flag I added). > Or do we still have code checking it somewhere? For compat, we should > probably keep checking for it wherever we used to... I'm unclear what you think that boils down to. It only meant something if you also specified URI_SAFE_FOR_UNTRUSTED_CONTENT, and in that case its meaning is now implied anyway. So I don't think we need to keep checking? Am I missing something? I could remove it from the idl, which in theory would break consumers. I just realized that pocket and mobile's about:reader thing both use it - I'll remove those, too, when landing. Add-ons MXR doesn't provide any hits.
Flags: needinfo?(bzbarsky)
> It only meant something if you also specified URI_SAFE_FOR_UNTRUSTED_CONTENT, > and in that case its meaning is now implied anyway. That's the part I was missing. OK, great. We can leave it, then, just document that setting it does nothing whatsoever.
Flags: needinfo?(bzbarsky)
Attachment #8741402 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/739482fdb6fe https://hg.mozilla.org/mozilla-central/rev/d4b74afcc60b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Group: firefox-core-security → core-security-release
Component: Untriaged → Networking
Product: Firefox → Core
Target Milestone: Firefox 48 → ---
Target Milestone: --- → mozilla48
Depends on: 1269238
Thanks for the hard work to make Mozilla more secure :) Wish you all well on your sides of the screen :) -Eric
Depends on: 1272139
Dan, are we OK to list the generic changes this bug made in "Firefox 48 for developers" type pages on MDN? When add-ons break, e.g. in bug 1272139, it would be helpful to have a public point of reference.
Flags: needinfo?(dveditz)
Keywords: addon-compat
I think we need to unhide this bug and explicitly explain what's going on because we're going to break stuff. At a quick glance over 50 add-ons reference URI_SAFE_FOR_UNTRUSTED_CONTENT and may need those linkable. I didn't dig so they might not all be about: pages, but some of them are. As to this bug specifically there are a few more we may want to be linkable: they are either likely to be linked from places like SUMO or they are well-known easter eggs. Supportish: about:credits about:license about:rights about:buildconfig about:logo (already linkable in this patch) Easter egg: about:mozilla about:robots I guess I don't care about the easter eggs, but I do worry that the others might be linked from non-chrome or support web pages.
Group: core-security-release
Flags: needinfo?(dveditz)
Blocks: 1273936
Filed bug 1273936 for the support-ish about: urls mentioned in the previous comment.
No longer blocks: 1273936
Depends on: 1273936
Alias: CVE-2016-5268
Whiteboard: [adv-main48+]
Depends on: 1293476
Depends on: 1308351
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: