Bug 609437 (CVE-2010-3771)

<isindex> doesn't CheckLoadURI

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
Layout: Form Controls
P1
normal
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: echo, Assigned: bz)

Tracking

({verified1.9.1, verified1.9.2})

unspecified
mozilla2.0b8
x86
Windows XP
verified1.9.1, verified1.9.2
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(blocking2.0 final+, blocking1.9.2 .13+, status1.9.2 .13-fixed, blocking1.9.1 .16+, status1.9.1 .16-fixed)

Details

(Whiteboard: [sg:critical (stepping stone)], URL)

Attachments

(3 attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.2.11) Gecko/20101012 Firefox/3.6.11

potential privilege escalation via elements without window 


Reproducible: Always

Steps to Reproduce:
1. open the redpil.html
2.
3.
Actual Results:  
Redirection to chrome://browser/content/broser.xul?

Expected Results:  
incrase privileges 
(but I do not know when it happens)

redpil.html

Comment 1

7 years ago
Created attachment 488081 [details]
redpil.html
Created attachment 488117 [details]
simpler testcase

The first opened/closed window was a red herring, all you need is an <isindex> element. You do seem to need to import it into the new document though. Are we forgetting to set the principal when we touch the new window? Testcase contains two variants that create a similar child window that works the way I'd expect (picks up the origin of the creating page).
Boris or Blake may know where we're going wrong in setting the new window's origin here. It's interesting that a web page can end up with a reference to a window containing a chrome:// url

I didn't try synthesizing events to see if we can avoid the need for the user to submit the prompt, but even if user interaction is required we could probably play clickjacking games or other social engineering to elicit it.

I expected a frame to work identically to the popup window, but doing this into a frame appears to work correctly. Maybe if the isindex element came from another window/document?

So far there's not much privilege escalation I can see -- you can't pick arbitrary chrome:// urls, it's just the main browser window, and once you have a handle to it you can no longer access that window (unless you find an additional bug in our same-origin/wrapper code, in which case the main problem is that additional bug).

Minefield appears immune. In the "broken" case here you just get an about:blank popup with no content. There's no exception or message on the error console about why the element wasn't appended to the popup (shouldn't there be?).
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

7 years ago
Minefield is showing a bug (filed elsewhere) where appending stuff to the DOM created by CreateAboutBlankContentViewer isn't creating frames for the nodes.  They're there in the DOM, if you look in DOM inspector....

Looking into the rest now.
(Assignee)

Comment 5

7 years ago
OK.  So the document involved has about:blank as URI, chrome://browser/content/browser.xul as the base URI (that would be a bug caused by the fix for bug 482659).  <isindex> submits to the base URI in this case because it has no action set.

<isindex> also has no CheckLoadURI check, which is a bug.  It should.  If it did, the load in this case would be denied and there would be no problem.  I did check that the principal of the <isindex> and its document is correct: it's the principal of the testcase page.

I'm going to post a patch to add the CheckLoadURI call.  We should file a separate bug on about:blank's base URI being wrong in this situation, I think.  And perhaps another bug on the fact that the code in nsDocument.cpp that does the baseURI channel property check doesn't then CheckLoadURI the result, unlike SetBaseURI?  Then again, we've been meaning to remove that last check, I think.
(Assignee)

Comment 6

7 years ago
Blake, for the base URI issue, wherever we change the principal is probably the right spot to generate a new about:blank URI... the problem is getting the right base URI there.

Note that on trunk we don't have this problem, because the CreateAboutBlankContentViewer call in nsGlobalWindow that we added for compartments makes the base URI about:blank in this situation.  It's not clear to me that this is right, fwiw; worth checking the spec.
(Assignee)

Comment 7

7 years ago
Created attachment 488126 [details] [diff] [review]
Fix
(Assignee)

Updated

7 years ago
Component: General → Layout: Form Controls
Product: Firefox → Core
QA Contact: general → layout.form-controls
(Assignee)

Comment 8

7 years ago
The patch was written + tested on 1.9.2, but it applies just fine to 1.9.1 and trunk, and is needed on both, I think.
Assignee: nobody → bzbarsky
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Priority: -- → P1
Whiteboard: [need review]
(Assignee)

Updated

7 years ago
Attachment #488126 - Flags: review?(jonas)
(Assignee)

Updated

7 years ago
Summary: privilege escalation → <isindex> doesn't CheckLoadURI
Comment on attachment 488126 [details] [diff] [review]
Fix

Glad this code is going away on m-c
Attachment #488126 - Flags: review?(jonas) → review+
(Assignee)

Updated

7 years ago
Attachment #488126 - Flags: approval2.0?
Attachment #488126 - Flags: approval1.9.2.13?
Attachment #488126 - Flags: approval1.9.1.16?
(Assignee)

Updated

7 years ago
Whiteboard: [need review] → [need approval]
(Assignee)

Updated

7 years ago
blocking2.0: ? → final+
(Assignee)

Updated

7 years ago
Attachment #488126 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Whiteboard: [need approval] → [need landing]
(Assignee)

Comment 10

7 years ago
Pushed http://hg.mozilla.org/mozilla-central/rev/4d26a3b6ade0
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
blocking1.9.1: ? → .16+
blocking1.9.2: ? → .13+
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Comment on attachment 488126 [details] [diff] [review]
Fix

Approved for 1.9.2.13 and 1.9.1.16, a=dveditz for release-drivers
Attachment #488126 - Flags: approval1.9.2.13?
Attachment #488126 - Flags: approval1.9.2.13+
Attachment #488126 - Flags: approval1.9.1.16?
Attachment #488126 - Flags: approval1.9.1.16+
(Assignee)

Comment 12

7 years ago
Pushed:

  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cc5b751b8be5
  http://hg.mozilla.org/releases/mozilla-1.9.1/rev/11d4251349a0
status1.9.1: wanted → .16-fixed
status1.9.2: wanted → .13-fixed
Whiteboard: [need landing]
(Assignee)

Comment 13

7 years ago
I filed bug 610001 on the base URI issue.
Whiteboard: [sg:critical (stepping stone)]
Verified fixed in 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.13pre) Gecko/20101117 Namoroka/3.6.13pre and in 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.16pre) Gecko/20101117 Shiretoko/3.5.16pre using Dan's testcase.
Keywords: verified1.9.1, verified1.9.2
Alias: CVE-2010-3771
Group: core-security
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.