User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; .NET CLR 1.0.3705; .NET CLR 1.1.4322) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113 I believe I have discovered a potential security vulnerability in Mozilla 1.6 and FireFox 0.8. It is likely that the bug also extends to other versions of Mozilla. The bug is a confidence-hack of sorts, allowing a malicious page to appear encrypted and present the certificate of another site. Reproducible: Always Steps to Reproduce: 1. Create a page on a non-SSL server that has a JavaScript or META redirect to the SSL-site whose certificate you want to spoof. 2. Your redirect should go to a script on that SSL-site that will redirect you somewhere else, and you need to be able to control where. 3. Have the SSL-site redirect the browser to an invalid domain (something syntactically invalid like http://a.b&c) 4. Use FireFox 0.8 to browse to the page you created in step 1. 6. An error will popup in FireFox. Click ok. 7. Your browser will still be on the original page from step 1 but the security- lock icon will still be on in the lower-left corner of FireFox. 8. Click the security icon and click the "View" button to view the certificate. 9. FireFox will present the certificate from the SSL-enabled site. I've created a simple test-case of the above scenario. I simply searched on Google for "redirect.cfm" to find any site with a common ColdFusion redirect script. I found emailfactory.com as the third result in Google. Then I created a page to redirect the browser to https://www.emailfactory.com/redirect.cfm?redirect=a.b&c. This makes emailfactory.com redirect the browser to a.b&c, which is invalid and causes Mozilla to remain on the current page and display a domain not found error. You can view this example at: http://www.ttar.org/test.html Actual Results: Mozilla's security icon remained on and clicking it showed the certificate of the spoofed SSL site. While there is an error popup notifying the user that Mozilla could not resolve the domain, that warning is likely not enough for the average user to realize that the security-icon and certificate visible on the page are invalid. Expected Results: The security icon shouldn't be on. This bug has also been emailed to security@mozilla.org

Darin, any thoughts on this?

To make the spoof complete, you should initially link to <https://www.emailfactory.com/redirect.cfm?redirect=www.ttar.org/test.html>. FYI, the attack does not work when you're behind a proxy, because the proxy will return (and Mozilla will display) an error page, replacing the old content.

Just an FYI -- Netscape Navigator 7.1 (and likely other versions) is also vulnerable. In Netscape, the security icon does not turn-on, but if you click on the "unlocked" security icon, it will still show the cert that belongs to the spoofed site. It's very likely that almost anybody embedding Mozilla needs to address this issue.

Not an NSS bug.

Adding PSM owner to CC list

With the seamonkey trunk, I get a dialog telling me that I am leaving an encrypted site and entering an unencrypted site. Next, I get a dialog telling me that the host a.b&c could not be found. Finally, I am left with the lock icon set and the old page content. So, this bug is definitely not Firefox specific. I'm collecting a nsSecureBrowserUI NSPR log now to see if I spot anything in there. I also confirmed this bug on the Mozilla 1.4 branch.

Most people quickly turn off the "entering" and "leaving" warning dialogs so they'd only see the host not found error. For bonus points make the invalid host a typical adserver name and people would actually be glad to see it thinking they're getting one less ad somewhere. combine with user@host spoof urls and phishing attacks become that much more effective. When I opened page info (in Mozilla) it said explicitly that the site www.ttar.org supports authentication for the page. Only if you view the certificate itself do you see it's a different site.

Things just got much worse: I’ve found a way to use this exploit without having the “host not found” dialog popup. It’s done using simple JavaScript. First, you call setTimeout() with about a one second timeout before you set document.location. Then, the function called by the timeout just calls window.stop(). If the timing is right, window.stop() will get called after the redirect has comeback from the spoofed site, but before the host not found dialog pops-up. The result is the same as the original exploit, but without the suspicious popup. I’ve combined the above with a much more convincing spoof. Try going to this URL and imagine that the “Click here” link on this page was placed wisely in a well-crafted (but fake) email: http://www.ttar.org/test_aol.html Notice that you’re not really at aol.com when you get the AOL main page – which contains a login form. However, unlike normal user@host spoofs, this one has the security-icon show-up as locked AND if you view the certificate, it shows that it belongs to America Online. This would likely convince any novice user that they could safely login to this page. For completeness I’ve changed the action URL on the login form to a CGI at ttar.org that will print out the username and password that was entered on the form. This should demonstrate the full potential of this bug.

Here's a log file that I generated by adding PR_LOG statements to nsDocLoaderImpl::OnSecurityChange. It shows that there is no call to OnSecurityChange for http://a.b&c/. I think one solution to this bug would be to suppress OnSecurityChange calls for requests that report a failure status, but I need to think about that some more to be sure.

Futher investigation reveals that this bug is triggered by the fact that nsHttpChannel makes calls to nsIProgressEventSink::OnProgress prior to processing its headers. As a result, the DocLoader observes the nsHttpChannel downloading content for the URL that results in a redirect. This causes the DocLoader to send STATE_TRANSFERRING notifications to its nsIWebProgressListener array. As a result, the nsSecureBrowserUI thinks that some content will be displayed for the https:// URL, and we end up with a lock icon. This can't be solved by the nsSecureBrowserUI since it cannot know (given our APIs) that the transferred data will actually never be displayed. Yes, it finds out later on that the URL was redirected (it sees a STATE_REDIRECTING event), but by then it is too late... some content was effectively (as far as its concerned) delivered to layout for rendering. I believe the correct solution to this bug is to suppress progress notification until the channel has received and processed its headers. I wrote a patch to do this and it works; however, there is one problem. We use OnProgress to cause the UI to be updated with progress of a form submission. That certainly corresponds to progress prior to receipt of response headers, and unfortunately our API does not provide an easy way for the observer to distinguish upload from download. Hmm.....

What we really need is a better API between Necko and the DocLoader... nsIProgressEventSink is so not sufficient! :-(

Actually, I can probably deal with the form submission problem by making DocLoader suppress OnStateChange from OnProgress until it receives an OnStatus with nsISocketTransport::STATUS_RECEIVING_FROM or nsITransport::STATUS_READING. Yeah, that might work well in absence of a better API.

On second thought, the form submission problem is perhaps less of an issue. In order to replicate this security bug using a form submission, the redirect would need to be of type 307 in order to preserve the HTTP method when following the redirect. However, a 307 requires user prompting, which we do. The user would therefore see a dialog asking them whether or not they wish to submit the contents of the form to the invalid URI. That may not be much protection. What do folks think? Should we worry about the form submission case? Tolga, can you modify your testcase so that it returns a 307 instead of a 302?

This patch is sufficient to fix the bug as reported. It likely does not protect against the same scenario applied to a form submission redirected via a 307 response. NOTE: while RFC 2616 states that a user-agent should preserve the request method in response to a 302, it also comments that most user-agents treat a 302 like a 303. Mozilla likewise behaves contrary to spec for historical consistency.

Per Darin's request: http://ttar.org/test_form.html This page contains a form which, when submitted, will submit to an https URL that returns a 307 to an invalid domain. Also, if you'd like to cook-up your own test cases, I've created a script on a SSL-enabled site that will redirect you to any URL with any status code: https://app.pbnext.com/test/redirect.em?code=CODE&url=URL For example: https://app.pbnext.com/test/redirect.em?code=302&url=http://a.b&c

I tried my new test-case and viewed the form-redirect popup as you'd indicated. I do not believe this is sufficient for novice users. It only states that your form submission has been redirected, and that's not that unusual. Old Netscape used to say that on POST to a 302 and people just ignored it. Also, Mozilla does not show the URL that the client is being redirected to. This means the person will never see that it's invalid.

I've considered Darin's solution and the form problem in more depth and: a) If you think about it, the security lock should actually be on during a POST while data is uploaded to a secure site. This makes sense because the content is in-fact being sent over SSL. It's arguable that the lock icon should also come on during a GET while the request is sent to the server -- since the request is sent over SSL. b) However when the redirect comes through, the lock icon should turn off -- before the redirection actually starts. c) It occurs to me that the lock icon should be turned off at the beginning off each hit (even internally generated "hits", like a redirect) and turned back on after an SSL connection has been established and the certificate verified, etc. d) I don't know much about the Mozilla object model, but it appears that nsSecureBrowserUi should not assume that a page is secure just because the URL is an https:// URL. The security-status of a page should be more explicitly returned to the UI from the channel. This way, it can be turned off at the beginning of each hit and then turned-back on only after an SSL connection has been established. e) Again, I'm not familiar with the Mozilla code. I知 making assumptions only based on the comments posted in this bug.

Comment on attachment 145762 [details] [diff] [review] v1 patch nevermind this patch. i found a problem with it.

So, the problem with the last patch was that it created the possibility that we wouldn't generate any progress notifications for a channel, and this in turn means that nsDocLoaderImpl wouldn't generate a STATE_TRANSFERRING event. That seems bad given the way the code in nsSecureBrowserUI works.

Here is a more involved patch. It solves both the GET and POST versions of this bug. I ended up synthesizing OnProgress events in nsHttpChannel:: OnDataAvailable since this was the best way to ensure that the progress events occur when they should (i.e., closest to where the data is actually handed to "layout"). I reworked nsHttpTransaction::OnTransportStatus with the changes to nsHttpChannel in mind. So, it now only sends status messages to the nsHttpChannel when necessary. I also had to make changes to nsDocLoaderImpl to make it reset its progress totals when switching from reporting upload progress to download progress. A side effect of these changes is that bug 237958 is also fixed (due to synthesized OnProgress in OnDataAvailable).

Oh, one other comment: I also changed nsDocLoaderImpl so that its OnProgress implementation only calls OnStateChange when data is first downloaded. Previously, as I have mentioned above, it would call OnStateChange(STATE_TRANSFERRING) whenever any progress was reported by the channel. However, we need to do that only in the download case and not in the upload case. So, this requirement translated to me adding a new field to nsRequestInfo to flag whether or not we are currently uploading (based on the previous call to OnStatus).

Comment on attachment 145782 [details] [diff] [review] v2 patch biesi: this is mostly what we talked about and then some ;-)

Comment on attachment 145782 [details] [diff] [review] v2 patch + PRBool mUploading; PRPackedBool, maybe? // if (aStatus) { + + // Remember the current status for this request the newline here seems unnecessary would be good if nsHttpTransaction used doxygen-compatible comment style for the member variables... and if it documented all of them :) ah well. looks good otherwise

the only thing I'm a bit worried about... what if you have an HTTPS file of zero length? You wouldn't send any OnProgress messages then... but does it matter? it's a bit of an edge case... I wonder what the uriloader/contentsink would do with such a request... hm...

(In reply to comment #24) > the only thing I'm a bit worried about... what if you have an HTTPS file of zero > length? You wouldn't send any OnProgress messages then... but does it matter? > it's a bit of an edge case... well, we currently do not send any progress for such a request.. and it doesn't seem like you would need to worry about showing the lock icon for it since there is no content for the user to see. but, maybe i'm missing something?

Comment on attachment 145782 [details] [diff] [review] v2 patch sr=bzbarsky, I guess.... All this stuff needs some documentation, deciding on exactly what the contracts are, and freezing...

bz: yeah, i agree. time willing, my plan is to document these interactions better and write up a testcase that can be added to tinderbox.

I left mUploading as a PRBool since converting to PRPackedBool wouldn't help the nsRequestInfo struct be any smaller. fixed-on-trunk for 1.7 final

Hmm... There was a bit of a Tp hit from this change. I guess it's us updating the status bar or something, but it may be something to look into.

(In reply to comment #29) > Hmm... There was a bit of a Tp hit from this change. I guess it's us updating > the status bar or something, but it may be something to look into. nsBrowserStatusFilter goes to some length to do that only rarely...

Yes, but there are some issues with the way it does do it (eg forcing sync reflow every time it does it) that could be problems here. Also, its definition of "rarely" is actually not that rare....

Couple possibilities: (1) we are firing DOCUMENT_START later to all webprogresslisteners, so we start tearing down the old document a bit later. (2) we are now reporting progress for cached loads (NOTE: this wouldn't affect UI since nsBrowserStatusHandler shows number of completed requests over total number of requests in progress meter. it would only matter when we are loading a single document.)

> (1) we are firing DOCUMENT_START later to all webprogresslisteners, so we start > tearing down the old document a bit later. nevermind this comment. DOCUMENT_START happens when the first request is added to the loadgroup, which happens in AsyncOpen calls. that's not related to this bug.

Compare http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1081626540.8740.gz to http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1081627500.16234.gz&fulltext=1 Most of the pages are not affected, really. A few got noticeably slowed (lxr page, quicken page, etc). The quicken page certainly has stuff it links to (at least on the web). The lxr page does not. So this could indeed be the cached loads issue (since many of those loads are cached, I would assume).

At the same time that I submitted this vulnerability to this mailing list and to bugzilla, I also dropped a note to CERT (knowing the CERT would obviously keep this confidential for now). CERT has since contacted me and asked several questions. I've updated them on the general status of this vulnerability and provided them with a copy of the bug history from Bugzilla. They also asked me: "Were you given any indications on the timeline for a fix for this issue? Likewise, did you have any expectations for publishing your findings personally? Our preference is give the vendor an opportunity to produce patches before we would publish something." I responded to the above question by telling CERT that I would coordinate with the Mozilla security team prior to releasing anything publicly -- assuming that such a process was complete by the first week in June. I picked this date very intentionally because, as far as I can, Mozilla 1.7 final is slated for release around May 21st. As you all know, this bug has already been fixed in 1.7. With that, I pose these questions to you: a) Is Mozilla planning to release a patch that current Mozilla users can use prior to the release of 1.7 final? My assumption is no. b) Does the patch that's already been put into CVS fix FireFox as well? Will others that embed Gecko need to make changes to their code, other than updating their Gecko engine? c) Does Mozilla have a list of projects embedding Gecko? Will those projects be notified by Mozilla? Do you require any assistance in that endeavor? d) Does Mozilla intend to release their own security announcement, other than what might be released by CERT or myself? e) Does Mozilla have a preference or policy regarding the release-date of this vulnerability? If asked to wait until after the release of 1.7 final, both CERT and I will. In either case, please provide some guidance on your preferences.

Answering just the questions I know answers to. Ccing some people who should be able to answer the rest. Also, you may want to pose those questions to security@mozilla.org, per http://www.mozilla.org/projects/security/security-bugs-policy.html > b) Does the patch that's already been put into CVS fix FireFox as well? Yes. > Will others that embed Gecko need to make changes to their code, other than > updating their Gecko engine? They will not.

Just to clarify -- I did post this to the security list as well. I just figured it'd be appropriate to post a copy here.

Adding Jon Granrose to CC list to help round up QA resources for verification

Comment on attachment 152720 [details] [diff] [review] Backport to 1.4 branch Darin, it would be great if you could take a look at this since I had to do some conflict resolution, especially in nsHttpTransaction::OnTransportStatus which looks substantially different than the version you patched against. I have verified this to plug the hole as demonstrated at http://www.ttar.org/test.html

Comment on attachment 152720 [details] [diff] [review] Backport to 1.4 branch sr=darin

Comment on attachment 152720 [details] [diff] [review] Backport to 1.4 branch a=blizzard for the 1.4.3 branch

Checked into the 1.4.3 branch

Removing security-sensitive flag for bugs on the known-vulnerabilities list

This patch appears to have caused a crash. See bug 242393.

Note: The Common Vulnerabilities and Exposures project (cve.mitre.org) has assigned the name CAN-2004-0761 to this issue.

Attaching testcases from www.ttar.org so we'll have them if that server ever goes away or gets cleaned up.