240053 - SSL Certificate Spoof -- Allows malicious page to present SSL certificate from another site

Status: RESOLVED FIXED

(Core :: Security, defect, P1)

Description

[Tolga Tarhan]

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 [e-mail]

Comment 1

[Johnny Stenback (:jst)]

Darin, any thoughts on this?

Comment 2

[Ben Bucksch (:BenB)]

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.

Comment 3

[Tolga Tarhan]

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.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Not an NSS bug.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Adding PSM owner to CC list

Comment 6

[Darin Fisher]

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.

Comment 7

[Daniel Veditz [:dveditz]]

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.

Comment 8

[Tolga Tarhan]

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. 

Comment 9

[Darin Fisher]

Attachment: log of calls to nsDocLoaderImpl::OnSecurityChange

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.

Comment 10

[Darin Fisher]

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.....

Comment 11

[Darin Fisher]

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

Comment 12

[Darin Fisher]

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.

Comment 13

[Darin Fisher]

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?

Comment 14

[Darin Fisher]

Attachment: v1 patch

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.

Comment 15

[Tolga Tarhan]

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

Comment 16

[Tolga Tarhan]

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.

Comment 17

[Tolga Tarhan]

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 18

[Darin Fisher]

Comment on attachment 145762 [details] [diff] [review]
v1 patch

nevermind this patch.  i found a problem with it.

Comment 19

[Darin Fisher]

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.

Comment 20

[Darin Fisher]

Attachment: v2 patch

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).

Comment 21

[Darin Fisher]

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 22

[Darin Fisher]

Comment on attachment 145782 [details] [diff] [review]
v2 patch

biesi: this is mostly what we talked about and then some ;-)

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

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

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

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...

Comment 25

[Darin Fisher]

(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 26

[Boris Zbarsky [:bzbarsky]]

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...

Comment 27

[Darin Fisher]

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.

Comment 28

[Darin Fisher]

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

Comment 29

[Boris Zbarsky [:bzbarsky]]

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.

Comment 30

[Christian :Biesinger (don't email me, ping me on IRC)]

(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...

Comment 31

[Boris Zbarsky [:bzbarsky]]

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....

Comment 32

[Darin Fisher]

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.)

Comment 33

[Darin Fisher]

> (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.

Comment 34

[Boris Zbarsky [:bzbarsky]]

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).

Comment 35

[Tolga Tarhan]

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.

Comment 36

[Boris Zbarsky [:bzbarsky]]

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
[e-mail], 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.

Comment 37

[Tolga Tarhan]

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.

Comment 38

[Daniel Veditz [:dveditz]]

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

Comment 39

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Backport to 1.4 branch

Comment 40

[Christopher Aillon (sabbatical, not receiving bugmail)]

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 41

[Darin Fisher]

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

sr=darin

Comment 42

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

a=blizzard for the 1.4.3 branch

Comment 43

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked into the 1.4.3 branch

Comment 44

[Daniel Veditz [:dveditz]]

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

Comment 45

[Darin Fisher]

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

Comment 46

[Mark Cox]

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

Comment 47

[Daniel Veditz [:dveditz]]

Attachment: original test.html from www.ttar.org

Comment 48

[Daniel Veditz [:dveditz]]

Attachment: Copy of test_form.html from www.ttar.org

Comment 49

[Daniel Veditz [:dveditz]]

Attachment: original test_aol2.html (target of next attachment)

Comment 50

[Daniel Veditz [:dveditz]]

Attachment: original test_aol.html modified to point at above

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

Comment 51

[amrinder]

This page contains a form which, when submitted, will submit to an https URL
that returns a 307 to an invalid domain. https://myaoldesktopgold.com/aol-gold-for-windows-and-mac/