609437 - (CVE-2010-3771) <isindex> doesn't CheckLoadURI

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P1)

Description

[echo]

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

[Bob Clary [:bc] (inactive)]

Attachment: redpil.html

Comment 2

[Daniel Veditz [:dveditz]]

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

Comment 3

[Daniel Veditz [:dveditz]]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 488126 [details] [diff] [review]
Fix

Glad this code is going away on m-c

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/4d26a3b6ade0

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 488126 [details] [diff] [review]
Fix

Approved for 1.9.2.13 and 1.9.1.16, a=dveditz for release-drivers

Comment 12

[Boris Zbarsky [:bzbarsky]]

Pushed:

  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cc5b751b8be5
  http://hg.mozilla.org/releases/mozilla-1.9.1/rev/11d4251349a0

Comment 13

[Boris Zbarsky [:bzbarsky]]

I filed bug 610001 on the base URI issue.

Comment 15

[Al Billings [:abillings - ex-MoCo]]

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.