Closed Bug 536466 (CVE-2010-2751) Opened 15 years ago Closed 14 years ago

SSL spoofing using location.href and location.reload() with history.back() and history.forward()

Categories

(Core Graveyard :: Security: UI, defect)

defect
Not set
major

Tracking

(blocking2.0 final+, blocking1.9.2 needed, status1.9.2 .7-fixed, blocking1.9.1 needed, status1.9.1 .11-fixed)

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- needed
status1.9.2 --- .7-fixed
blocking1.9.1 --- needed
status1.9.1 --- .11-fixed

People

(Reporter: jordi.chancel, Assigned: mayhemer)

References

()

Details

(Keywords: fixed1.9.0.20, verified1.9.1, verified1.9.2, Whiteboard: [sg:moderate spoof][3.6.x])

Attachments

(7 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; fr; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; fr; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6

When you enter an address with an automatic redirection enabled on an other adress (with SSL certificat) , 
enter again the same adress( with redirection disabled) , 
actualize again the same page (with automatic redirection enabled),
history.back() & history.forward().

Reproducible: Always

Steps to Reproduce:
Step 1=> enter an adress with an automatic redirection on an https website with SSL cetificat 
Step 2=> reopen the same adress but this time with redirection disabled
Step 3=> actualize (the redirection must be re-enabled automaticatly)
Step 4=> Go back & forward

or

Step 1=> Create an html file PoC with a special javascript and an other file with an activation/desactivation of the automatic rediction faith on two
Step 2=> Open the page
Actual Results:  
The adress (with redirection enabled/disabled) takes content and SSL certificat of the HTTPS website
Attached image screen shot (obsolete) —
Summary: location's bar content on a page with SSL certificat can be spoofed with a valid certificat → the content of location's bar on a page with SSL certificat can be spoofed with a valid certificat
Whiteboard: [sg:moderate] spoof
Component: General → Location Bar and Autocomplete
QA Contact: general → location.bar
Attached file link testcase (obsolete) —
link of testcase ( http://www.alternativ-testing.fr/5df49s6q4s8d5zq-firefoxSSL/ssl3.php )
Attachment #418910 - Attachment is obsolete: true
Attached file Link Testcase2
Attachment #419123 - Attachment is obsolete: true
document.location and SSL indicat of https website(or http) is replaced by the document.location of redirection page(enabled/disabled faith on two)
Attachment #419145 - Attachment description: Testcase part1 alert(document.location) → Testcase2 part1 alert(document.location)
Whiteboard: [sg:moderate] spoof → [sg:moderate] spoof
Confirmed using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091230 Minefield/3.7a1pre.

Need a better summary...
Assignee: nobody → kaie
Status: UNCONFIRMED → NEW
blocking1.9.1: --- → ?
blocking2.0: --- → ?
Component: Location Bar and Autocomplete → Security: UI
Ever confirmed: true
Flags: blocking1.9.0.18?
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: location.bar → ui
Hardware: x86 → All
Summary: the content of location's bar on a page with SSL certificat can be spoofed with a valid certificat → SSL spoofing using location.href and location.reload() with history.back() and history.forward()
Whiteboard: [sg:moderate] spoof → [sg:moderate spoof]
Flags: blocking1.9.2?
Somewhat shades of bug 514232.  Honza, do you want to take a look at this?
And more interestingly, is this a regression from, say, 1.9.0?
(In reply to comment #8)
> Somewhat shades of bug 514232.  Honza, do you want to take a look at this?

Yes, I'm going to analyze this.
Assignee: kaie → honzab.moz
Status: NEW → ASSIGNED
Flags: blocking1.9.2? → blocking1.9.2-
Whiteboard: [sg:moderate spoof] → [sg:moderate spoof][3.6.x]
So, the only problem I can see here is that the URL in the address bar is replaced with the address of the page that leads to the target of the redirect. The same is then displayed in the Page info dialog.

It seems that security checks are made correctly against the 'https://target-of-redirect' principal. There is no way to modify the content directly via document.write or so.

I have an automated test.
(In reply to comment #9)
> And more interestingly, is this a regression from, say, 1.9.0?

Confirmed with Gran Paradiso, 3.0.17pre.
I think this is not an NSS issue.
> Confirmed with Gran Paradiso, 3.0.17pre.

I should have been clearer.  I meant 1.9.0.0 and in particular something from before we fixed bug 514232.
Optimistically putting this into 1.9.0.18/1.9.8 assuming that will match up with the 1.9.2.1, but we might have to adjust later.
blocking1.9.1: ? → .8+
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.18?
Flags: blocking1.9.0.18+
(In reply to comment #14)
> > Confirmed with Gran Paradiso, 3.0.17pre.
> 
> I should have been clearer.  I meant 1.9.0.0 and in particular something from
> before we fixed bug 514232.

I have backed out http://hg.mozilla.org/mozilla-central/rev/05af3507e9c5 and 59ca9e3e4ef9 and this bug persists. So it seems to be older.
Test case ones again:
1. open window with URL of a page that redirects (redirect works now) -> we are on the redirect *target* page; first SHE created
2. do location.href = URL of a page that redirects (redirect now doesn't work) -> we are back at the page that redirects; second SHE created
3. do location.reload() (redirect now works) -> we get to the redirect *target* page; still using the second SHE

BUG => In case of location.reload() load type is LOAD_CMD_RELOAD and this code

http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8874

doesn't update history, the current SH entry (the second one in this case) is not updated with the new URI of the redirect target. On back and forward we have the correct content but we update the session with the old URL.

Call stack this code gets invoked:

>	docshell.dll!nsDocShell::OnNewURI(nsIURI * aURI=0x06a62078, nsIChannel * aChannel=0x069dc3c0, nsISupports * aOwner=0x00000000, unsigned int aLoadType=2, int aFireOnLocationChange=0, int aAddToGlobalHistory=1)  Line 8852	C++
 	docshell.dll!nsDocShell::OnLoadingSite(nsIChannel * aChannel=0x069dc3c0, int aFireOnLocationChange=0, int aAddToGlobalHistory=1)  Line 8969 + 0x29 bytes	C++
 	docshell.dll!nsDocShell::CreateContentViewer(const char * aContentType=0x0629a078, nsIRequest * request=0x069dc3c0, nsIStreamListener * * aContentHandler=0x06657a70)  Line 7043 + 0x15 bytes	C++
 	docshell.dll!nsDSURIContentListener::DoContent(const char * aContentType=0x0629a078, int aIsContentPreferred=0, nsIRequest * request=0x069dc3c0, nsIStreamListener * * aContentHandler=0x06657a70, int * aAbortProcess=0x0013d5bc)  Line 138 + 0x20 bytes	C++
 	docshell.dll!nsDocumentOpenInfo::TryContentListener(nsIURIContentListener * aListener=0x069154c0, nsIChannel * aChannel=0x069dc3c0)  Line 736 + 0x41 bytes	C++
 	docshell.dll!nsDocumentOpenInfo::DispatchContent(nsIRequest * request=0x069dc3c0, nsISupports * aCtxt=0x00000000)  Line 434 + 0x39 bytes	C++
 	docshell.dll!nsDocumentOpenInfo::OnStartRequest(nsIRequest * request=0x069dc3c0, nsISupports * aCtxt=0x00000000)  Line 280 + 0x10 bytes	C++
 	necko.dll!nsHttpChannel::CallOnStartRequest()  Line 839 + 0x47 bytes	C++
 	necko.dll!nsHttpChannel::ProcessNormal()  Line 1145 + 0x8 bytes	C++
 	necko.dll!nsHttpChannel::ProcessResponse()  Line 1014 + 0x8 bytes	C++
 	necko.dll!nsHttpChannel::OnStartRequest(nsIRequest * request=0x05f1b1b8, nsISupports * ctxt=0x00000000)  Line 5162 + 0xb bytes	C++


At the moment no idea for a fix. Probably change mLoadType from RELOAD to NORMAL in case we detect a redirect?
Attached patch v1 (obsolete) — Splinter Review
This is my proposed fix for consideration.  There is a single drawback, when we set location to a page leading to a redirect, and redirect doesn't work, and later we refresh the page while redirect does work, there is created an SH entry with address of the page leading to the redirect.

I'm not sure how serious that is.
Attachment #422570 - Flags: review?(bzbarsky)
Smaug suggested to test with IE, so the result is that on refresh in step 3 of the test IE8 loads the content of the redirect target page, updates the security indicators to show an encrypted site and updates the URL in the address bar and doesn't crate a new history entry and either doesn't blow away forward history.

It also doesn't refresh the page (=is not doing a server request) on going back/forward.

I found an IE bug: when you do a test in a secondary open tab, the address bar is not updated after refresh, it is refreshed after back/forward or switch to another tab and then going back; just a note...

Going to study smaug's patch changing some SHE manipulation and potentially base a new patch up on his one.
Readjusting flags; we'll get this next release.
blocking1.9.1: .8+ → needed
blocking1.9.2: --- → ?
Flags: blocking1.9.0.18+ → blocking1.9.0.19?
Whiteboard: [sg:moderate spoof][3.6.x] → [sg:moderate spoof][3.6.x][needs r=bz]
Whiteboard: [sg:moderate spoof][3.6.x][needs r=bz] → [sg:moderate spoof][3.6.x]
Comment on attachment 422570 [details] [diff] [review]
v1

This patch is wrong, I'm working on a different approach atm.
Attachment #422570 - Attachment is obsolete: true
Attachment #422570 - Flags: review?(bzbarsky)
Attached patch branch v0.9 (obsolete) — Splinter Review
This is preview of my solution for branches: cancel the redirect when we reload or navigate.  The patch is based on m-c, merging is needed.
Attachment #423783 - Flags: review?(bzbarsky)
Attached patch v2Splinter Review
First, I was looking at Olli's patch to find API that I could use and I haven't found anything.

Then I spotted code handling LOAD_NORMAL_REPLACE load type here [1]. This piece of code and the LOAD_NORMAL_REPLACE type does exactly what I need.  This doesn't affect iframes, but this bug could not be reproduced when manipulating an iframe.

This is IMHO also a branch safe patch.

Also tested on top of patch for bug 462076, everything works.

[1] http://hg.mozilla.org/mozilla-central/annotate/5f631340007c/docshell/base/nsDocShell.cpp#l9098
Attachment #423827 - Flags: review?(bzbarsky)
Needed for 1.9.2, not blocking for 1.9.0 (since that branch is going out of support soon) but would definitely want the patch there when it gets reviewed.
blocking1.9.2: ? → needed
Flags: blocking1.9.0.19? → blocking1.9.0.19-
Severity: normal → major
Comment on attachment 423827 [details] [diff] [review]
v2

r=bzbarsky.  Nice approach!  Really sorry for the lag...

Do you need the v0.9 branch thing reviewed still, or do we just want to do this on branches?
Attachment #423827 - Flags: review?(bzbarsky) → review+
Comment on attachment 423783 [details] [diff] [review]
branch v0.9

I don't think we need this patch.
Attachment #423783 - Attachment is obsolete: true
Attachment #423783 - Flags: review?(bzbarsky)
Resolved for Firefox 3.6.4 or another ?
Attachment #423827 - Flags: superreview?(cbiesinger)
Apparently, the ball was dropped here. Honza, poke?
status1.9.2: --- → ?
status2.0: --- → ?
Comment on attachment 423827 [details] [diff] [review]
v2

+    if (~(aRedirectFlags & nsIChannelEventSink::REDIRECT_INTERNAL) && 

do you mean ! instead of ~?

sr=biesi
Attachment #423827 - Flags: superreview?(cbiesinger) → superreview+
Is this ready to land on trunk?
I'll address comment 29 and land.  Probably on Wednesday.
Attached patch Tests for v2Splinter Review
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #444675 - Flags: approval1.9.2.5?
Attachment #444675 - Flags: approval1.9.1.11?
Attachment #444675 - Flags: approval1.9.2.5?
Attachment #444675 - Flags: approval1.9.2.5+
Attachment #444675 - Flags: approval1.9.1.11?
Attachment #444675 - Flags: approval1.9.1.11+
Flags: in-testsuite?
Attachment #444675 - Flags: approval1.9.2.5+ → approval1.9.2.6+
Comment on attachment 444675 [details] [diff] [review]
v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35] [Check-in 1.9.0 comment 42]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/933ad42c0b77
Attachment #444675 - Attachment description: v2 as landed [Checkin comment 32] → v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34]
Attachment #444675 - Flags: approval1.9.0.next?
Attachment #444675 - Attachment description: v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] → v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35]
Comment on attachment 444675 [details] [diff] [review]
v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35] [Check-in 1.9.0 comment 42]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0fb56c49d1e9
The original STR is now 404. Can someone provide a link for manual verification since the tests aren't running yet?
Whiteboard: [sg:moderate spoof][3.6.x] → [sg:moderate spoof][3.6.x] [qa-examined-192] [qa-needs-STR]
(In reply to comment #36)
> The original STR is now 404. Can someone provide a link for manual verification
> since the tests aren't running yet?

http://www.alternativ-testing.fr/6df4d587d9s-firefoxdocument-locationpersistbug/documentlocationfirefox.php
(In reply to comment #37)
> (In reply to comment #36)
> > The original STR is now 404. Can someone provide a link for manual verification
> > since the tests aren't running yet?
> 
> http://www.alternativ-testing.fr/6df4d587d9s-firefoxdocument-locationpersistbug/documentlocationfirefox.php

Used this to verify for 1.9.1 and 1.9.2. SSLspoofing doesn't work in 1.9.1.11 build 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.11) Gecko/20100701 Firefox/3.5.11 (.NET CLR 3.5.30729)) or 1.9.2.7 build 1 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7) Gecko/20100701 Firefox/3.6.7 (.NET CLR 3.5.30729)). Spoofing clearly occurs in 1.9.1.10 and 1.9.2.6.
Whiteboard: [sg:moderate spoof][3.6.x] [qa-examined-192] [qa-needs-STR] → [sg:moderate spoof][3.6.x]
Alias: CVE-2010-2751
Group: core-security
Comment on attachment 444675 [details] [diff] [review]
v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35] [Check-in 1.9.0 comment 42]

Approved for 1.9.0.20, a=dveditz
Attachment #444675 - Flags: approval1.9.0.next? → approval1.9.0.next+
Honza, do you mind if I go ahead and check this in for you on 1.9.0?
(In reply to comment #40)
> Honza, do you mind if I go ahead and check this in for you on 1.9.0?

Feel free, but please test the patch works, i.e. run test case in comment 37 w/o the patch first and then w/ the patch to see the problem is fixed with the patch.
(In reply to comment #41)
> Feel free, but please test the patch works, i.e. run test case in comment 37
> w/o the patch first and then w/ the patch to see the problem is fixed with the
> patch.

Yep, I did that before I started working on the collection of 1.9.0 bugs :) (which is a good thing, because the testcase in comment 37 is 404 again).

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.919; previous revision: 1.918
done
Keywords: fixed1.9.0.20
(In reply to comment #42)
> (In reply to comment #41)
> > Feel free, but please test the patch works, i.e. run test case in comment 37
> > w/o the patch first and then w/ the patch to see the problem is fixed with the
> > patch.
> 
> Yep, I did that before I started working on the collection of 1.9.0 bugs :)
> (which is a good thing, because the testcase in comment 37 is 404 again).
> 
> Checking in docshell/base/nsDocShell.cpp;
> /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
> new revision: 1.919; previous revision: 1.918
> done

http://www.alternativ-testing.fr/6df4d587d9s-testcasefirefoxdocument-location201001/documentlocationfirefox.php
Smokey, thank you.
Attachment #444675 - Attachment description: v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35] → v2 as landed [Checkin comment 32] [Checkin 1.9.1 comment 34] [Checkin 1.9.2 comment 35] [Check-in 1.9.0 comment 42]
blocking2.0: ? → final+
status2.0: ? → ---
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: