Last Comment Bug 253121 - lock icon and certificates spoofable with onunload document.write
: lock icon and certificates spoofable with onunload document.write
Status: VERIFIED FIXED
: fixed-aviary1.0, fixed1.4.3, fixed1.7.2, fixed1.7.5
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- critical with 1 vote (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Robin Monks
Mentors:
http://www.cipher.org.uk/index.php?p=...
: 253215 253285 253424 (view as bug list)
Depends on:
Blocks: lockicon
  Show dependency treegraph
 
Reported: 2004-07-26 10:22 PDT by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2006-03-23 05:13 PST (History)
29 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (297 bytes, text/html)
2004-07-26 10:23 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details
Better testcase (325 bytes, text/html)
2004-07-26 10:54 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details
Proposed fix. Still needs more testing, but appears to fix the bug. (11.16 KB, patch)
2004-07-26 17:39 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Same as above with code to carry security info over from the caller of document.open(). (12.87 KB, patch)
2004-07-27 13:47 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Same as above, but don't forget the security info in a call to document.open() from the same document. (12.99 KB, patch)
2004-07-27 14:10 PDT, Johnny Stenback (:jst, jst@mozilla.com)
darin.moz: review+
bzbarsky: superreview+
Details | Diff | Review
Final patch for checkin (aviary branch) (11.29 KB, patch)
2004-07-27 15:52 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Final patch for checkin (aviary branch) (12.98 KB, patch)
2004-07-27 17:21 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Final patch for checkin (trunk) (14.35 KB, patch)
2004-07-27 17:25 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
For 1.4 branch (9.59 KB, patch)
2004-07-27 23:47 PDT, Christopher Aillon (sabbatical, not receiving bugmail)
caillon: approval1.4.3+
Details | Diff | Review

Description :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 10:22:49 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1

Testcase will be attached 

Reproducible: Always
Steps to Reproduce:
1. Load testcase (attachment coming soon)
2. Right click page -> View Page Info, go to security tab
3. Notice that the site appears to be secure, and the certificate from
https://shellcity.net is shown




See also
Comment 1 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 10:23:29 PDT
Created attachment 154373 [details]
Testcase

Demonstrates spoof.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2004-07-26 10:35:33 PDT
I get at least 4 dialogs warning me about this certificate, at least one of
which says that this isn't the right cert for this site.  Is this really an issue?

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040626 Firefox/0.9.1
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 10:49:29 PDT
That message appears because of a misconfiguration on the shellcity.net site,
which is unrelated to this bug. I picked that site as it was the first I could
think of that had SSL set up. If you can think of another site where ssl is
setup properly you can substitute the URL in the testcase.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 10:54:35 PDT
Created attachment 154374 [details]
Better testcase

"at least one of which says that this isn't the right cert for this site.  Is
this really an issue?"

This attachment should make the site appear without any warnings.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2004-07-26 11:29:37 PDT
Okay, that testcase works.  I get the lock icon.  This still isn't a really big
issue, but it should be fixed.
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 11:38:55 PDT
This is a very big issue. People rely on that icon to determine whether or not
the site is "safe". I could make a fake paypal site and use that technique to
make it look as though it is the real paypal site, and collect credit card
numbers. These certificates are critical, their entire purpose is to confirm
that the site you *are* dealing with is in fact the site you *think* you are
dealing with. This vulnerability allows someone to completely bypass this
security measure.
Comment 7 Jesse Ruderman 2004-07-26 12:33:12 PDT
When I view the testcase, I get a lock icon (incorrect and bad), but I see
"bugzilla.mozilla.org" in the URL bar and when I double-click the lock icon.
Comment 8 Ted Mielczarek [:ted.mielczarek] 2004-07-26 12:36:33 PDT
Stating the obvious: has anyone tested this on seamonkey?

Also, from Jesse's comment, does this really need to be "critical"?  You have to
actually go to "View Certificate" to see the spoofed site's name.  The only
thing actually being spoofed here is the lock icon.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-07-26 12:39:04 PDT
jruderman@gmail.com: 
That is the one spot where the correct site is actually listed. Try clicking
"View" from the security tab to see the full cert. Result: the paypal info is
displayed.

luser_bugzilla@perilith.com:
The actual cert itself is spoofed, so this is more than a cosmetic issue. I am
assuming that the reason that b.m.o is shown in the initial dialog is because
that information is retrieved independantly of the cert. The cert details are
still wrong.
Comment 10 logan 2004-07-26 12:41:20 PDT
OS -> All

and yeah, this affects Mozilla as well
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-26 15:32:40 PDT
Taking bug.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-26 17:39:41 PDT
Created attachment 154414 [details] [diff] [review]
Proposed fix. Still needs more testing, but appears to fix the bug.
Comment 13 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-26 17:40:46 PDT
Comment on attachment 154414 [details] [diff] [review]
Proposed fix. Still needs more testing, but appears to fix the bug.

Darin, does this look good so far?
Comment 14 Darin Fisher 2004-07-26 18:33:27 PDT
> Darin, does this look good so far?

I haven't reviewed the entire patch, but yeah... major oops that we aren't
propogating the securityInfo from the original channel to the wyciwyg channel :-(
Comment 15 Martin Creutziger [:MMx] 2004-07-26 23:17:00 PDT
*** Bug 253215 has been marked as a duplicate of this bug. ***
Comment 16 Martin Creutziger [:MMx] 2004-07-26 23:24:33 PDT
Can confirm this behaviour also in MacOS X with the following builds:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de-DE; rv:1.7) Gecko/20040628
Firefox/0.9.1
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040720
Firefox/0.9.1+
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de-AT; rv:1.7) Gecko/20040616

Platform->All

Changing Product to Browser, it also affects Mozilla
Comment 17 Malcolm Rowe 2004-07-27 00:08:23 PDT
Comment on attachment 154414 [details] [diff] [review]
Proposed fix. Still needs more testing, but appears to fix the bug.

>+    // document.open() called from another window, loose our cached
>+    // security info

"lose". (or "flush" or something else).
Comment 18 Bill Mason 2004-07-27 08:49:44 PDT
*** Bug 253285 has been marked as a duplicate of this bug. ***
Comment 19 dovix 2004-07-27 09:10:05 PDT
(In reply to comment #18)
> *** Bug 253285 has been marked as a duplicate of this bug. ***

I could not find a matching bug report, so I got worried for a moment. Glad
you're already working on this. 

Here's a summary from the bug I opened, just in case it might help:

---------------------

I got this link from a post in some Israeli forum. The sample spoofing page was
provided by the person who wrote that post (aparently not somebody who likes
Mozilla).

The link for the full-disclosure message is:

http://lists.netsys.com/pipermail/full-disclosure/2004-July/024372.html

That person also provided a test page for the issue (sorry for the message
there, as I mentioned that person does not seem to be a Mozilla lover):

http://uploaded.fresh.co.il/2004/07/27/303883.html

Comment 20 Darin Fisher 2004-07-27 12:14:13 PDT
Comment on attachment 154414 [details] [diff] [review]
Proposed fix. Still needs more testing, but appears to fix the bug.

r=darin

this patch makes sense to me, though i'm not sure i fully understand the code
in OpenCommon.	jst: can you explain that part of the patch?
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 12:20:29 PDT
(In reply to comment #20)
> (From update of attachment 154414 [details] [diff] [review])
> r=darin
> 
> this patch makes sense to me, though i'm not sure i fully understand the code
> in OpenCommon.	jst: can you explain that part of the patch?
> 

I still need to test that, but the idea is to forget the security information we
got from the channel when some *other* window calls document.open() on this
document (anyone can call document.open(), no same-origin checks done there).

IOW, if I do:

  w=window.open("http://www.paypal.com");

w will open and show a locked icon, if I then do:

  w.document.open();
  w.document.write("not secure any more");
  w.document.close();

we should make sure we do *not* display the lock icon any more.

Needs testing still.
Comment 22 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 12:47:19 PDT
Ok, tested that and that case works now too. But after chatting with bz we
decided to change this a bit and in stead of just dropping our reference to our
security info when document.write is called etc, we'll grab the security info
from the calling document in stead, that way, if a secure site does
document.open() on a document that's not secure, it'll become secure (and the
URL bar will reflect the source of the document.open() call etc). New patch
coming up...
Comment 23 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 13:47:07 PDT
Created attachment 154499 [details] [diff] [review]
Same as above with code to carry security info over from the caller of document.open().
Comment 24 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 13:49:38 PDT
Comment on attachment 154499 [details] [diff] [review]
Same as above with code to carry security info over from the caller of document.open().

Requesting reviews. Darin, would you have a look at the additional changes,
it's mostly the same as before, and let me know if my explanation didn't make
sense.
Comment 25 Darin Fisher 2004-07-27 13:53:36 PDT
> we'll grab the security info
> from the calling document in stead, that way, if a secure site does
> document.open() on a document that's not secure, it'll become secure (and the
> URL bar will reflect the source of the document.open() call etc).

wouldn't "mixed-content" make more sense?  calling the content secure when it
may not be entirely secure seems somewhat wrong.
Comment 26 Darin Fisher 2004-07-27 13:55:29 PDT
> wouldn't "mixed-content" make more sense?  calling the content secure when it
> may not be entirely secure seems somewhat wrong.

oh, but wait... no vestige of the old document remains in this case, right?
Comment 27 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 14:08:56 PDT
(In reply to comment #26)
> > wouldn't "mixed-content" make more sense?  calling the content secure when it
> > may not be entirely secure seems somewhat wrong.
> 
> oh, but wait... no vestige of the old document remains in this case, right?

Right.
Comment 28 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 14:10:44 PDT
Created attachment 154505 [details] [diff] [review]
Same as above, but don't forget the security info in a call to document.open() from the same document.
Comment 29 Boris Zbarsky [:bz] 2004-07-27 14:21:47 PDT
Comment on attachment 154505 [details] [diff] [review]
Same as above, but don't forget the security info in a call to document.open() from the same document.

sr=bzbarsky
Comment 30 Darin Fisher 2004-07-27 14:47:45 PDT
Comment on attachment 154505 [details] [diff] [review]
Same as above, but don't forget the security info in a call to document.open() from the same document.

r=darin, but please add a comment about Reset to explain the securityInfo
juggling in OpenCommon.
Comment 31 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 15:52:18 PDT
Created attachment 154516 [details] [diff] [review]
Final patch for checkin (aviary branch)
Comment 32 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 17:14:26 PDT
Comment on attachment 154516 [details] [diff] [review]
Final patch for checkin (aviary branch)

This diff was missing the security/ changes, new diff coming up.
Comment 33 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 17:21:48 PDT
Created attachment 154525 [details] [diff] [review]
Final patch for checkin (aviary branch)
Comment 34 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 17:25:59 PDT
Created attachment 154526 [details] [diff] [review]
Final patch for checkin (trunk)
Comment 35 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 17:28:05 PDT
Fixed on trunk and aviary branch.
Comment 36 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 17:47:03 PDT
For the record, the aviary branch diff applies cleanly against the current 1.7
branch.
Comment 37 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-07-27 18:27:16 PDT
I think you should land this on the 1.7 branch if you haven't already
Comment 38 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 21:07:51 PDT
Fixed on the 1.7 branch too.
Comment 39 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-27 22:31:27 PDT
Caillon pointed out that I forgot to rev the IIDs for nsIWyciwygChannel and
nsIDocument, so I did that on all branches.
Comment 40 Christopher Aillon (sabbatical, not receiving bugmail) 2004-07-27 23:47:54 PDT
Created attachment 154539 [details] [diff] [review]
For 1.4 branch
Comment 41 Christopher Aillon (sabbatical, not receiving bugmail) 2004-07-27 23:50:50 PDT
Comment on attachment 154539 [details] [diff] [review]
For 1.4 branch

a=blizzard for 1.4.3
Comment 42 OstGote! 2004-07-28 01:27:27 PDT
(In reply to comment #38)
> Fixed on the 1.7 branch too.

Nit-picking: Keyword should be fixed1.7.2, am i right? :-)
Comment 43 Boris Zbarsky [:bz] 2004-07-28 09:17:07 PDT
*** Bug 253424 has been marked as a duplicate of this bug. ***
Comment 44 Boris Zbarsky [:bz] 2004-07-28 09:18:02 PDT
This may be worth doing security releases of the branches, especially given the
exposure it got....
Comment 45 Derek McCaw 2004-07-29 10:28:27 PDT
(In reply to comment #44)
> This may be worth doing security releases of the branches, especially given the
> exposure it got....

Agreed. I just ran across it on bugtraq, and it could be nice publicity to have
this kind of response time... though it would be nice if we had an easy patching
system going.
Comment 46 logan 2004-07-29 10:34:56 PDT
In addition to this and bug 249004, it may also be nice to nail bug 250906 as
well. Having all of these taken care of in one release would make many happy,
I'm sure.
Comment 47 Johnny Stenback (:jst, jst@mozilla.com) 2004-07-29 14:55:14 PDT
Fixed on the 1.7.2 *branch* now too.
Comment 48 dovix 2004-07-30 11:19:27 PDT
(In reply to comment #46)
> In addition to this and bug 249004, it may also be nice to nail bug 250906 as
> well. Having all of these taken care of in one release would make many happy,
> I'm sure.

Here's another new security issue that was just opened: 253745

It was reported today by Secunia as "Moderately critical".






Comment 49 David E. Ross 2004-07-30 14:11:11 PDT
Bug #253745 is a duplicate of bug #252198.  
Comment 50 Tracy Walker [:tracy] 2004-08-02 11:46:43 PDT
looks good with seamonkey 1.7.2 build 2004-07-31-16-1.7.2
Comment 51 sairuh (rarely reading bugmail) 2004-08-02 12:45:07 PDT
per comment 50, ditto with linux seamonkey 1.7.2, 2004080114 build.
Comment 52 Marcia Knous [:marcia - use ni] 2004-08-02 14:39:16 PDT
Mac build 1.7.2 20040731 looks good in this regard.
Comment 53 sairuh (rarely reading bugmail) 2004-08-02 15:39:09 PDT
mac firefox 2004080211-0.9.3: due to bug 244479 I cannot view the Security tab
in Page Info. however, no locked padlock appears in the status bar with the test
case, so there's no false indication that a secure site has loaded.
Comment 54 Mark Cox 2004-08-03 00:51:36 PDT
Note: The Common Vulnerabilities and Exposures project (cve.mitre.org) has
assigned the name CAN-2004-0763 to this issue.
Comment 55 :Gavin Sharp [email: gavin@gavinsharp.com] 2004-08-15 13:55:20 PDT
Verifying per comment 50 to comment 53.
Comment 56 Ben Fowler 2004-09-28 06:33:30 PDT
The bug title has "onunload" in it.

Would it be useful to have the onunload( ) hook disabled by default? It
doesn't have many positive uses.

In other words onunload handlers in web content would not be run unless
the user had gone to the trouble of enabling this feature ...
Comment 57 Martin Creutziger [:MMx] 2004-09-28 06:42:02 PDT
(In reply to comment #56)

Please note that this bug is VERIFIED FIXED.
I don't know if any further action is required.
Comment 58 Ben Fowler 2006-03-16 06:46:58 PST
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060316 Firefox/1.6a1

The testcase in comment 4 shows:

  1. https://www.paypal.com/ in the URL bar
  2. A black padlock at the right of the status bar
  3. www.paypal.com  at the right of the status bar
  4. A blank page

There was a transient title of 'Spoofer' and some flickering text saying "Enter
...", before the blank page was fully displayed.

Given comment 53 , it might be a good idea for somebody else to check on the
padlock icon and the status bar - perhaps I am on the wrong lines somewhere
and/or need a new profile!
Comment 59 Daniel Veditz [:dveditz] 2006-03-17 21:12:43 PST
Ben: I cannot reproduce using a trunk or 1.8 branch build on windows. Can you try on a new profile? Is there someone else who can try this on a Mac?
Comment 60 Jesse Ruderman 2006-03-17 21:30:44 PST
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060317 Firefox/1.6a1

Loading the testcase in comment 4, I see "Insert fake paypal site here. See security icon in lower left.", but the URL bar and status bar indicate that I'm on bugzilla.mozilla.org.

Ben, do you see chrome JS errors in the JavaScript console when you load it?  (You'll have to set javascript.options.showInConsole in about:config to true before testing.)
Comment 61 Ben Fowler 2006-03-18 05:53:50 PST
Irrespective of the javascript config setting, I get this message:

Error: uncaught exception: [Exception... "Component returned failure code: 0x804b003d [nsIDOMNSLocation.reload]"  nsresult: "0x804b003d (<unknown>)"  location: "JS frame :: https://bugzilla.mozilla.org/attachment.cgi?id=154374&action=view :: onunload :: line 1"  data: no]

When I turn on that setting, I see the same as you do. Turning the setting off,
I can reproduce what I said in comment 58 . I will try with a nightly.
Comment 62 Ben Fowler 2006-03-23 05:13:38 PST
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20060321 Firefox/2.0a1

> When I turn on that setting, I see the same as you do. Turning the setting off,
> I can reproduce what I said in comment 58 . I will try with a nightly.

Testing with the Bon Echo alpha (and a new profile) shows the "Insert fake paypal site here. See security icon in lower left." message in full, but the status
bar correctly says "bugzilla.mozilla.org". Nothing in the javascript console, and in general the same as  comment 60 .

Note You need to log in before you can comment on or make changes to this bug.