447579 - (CVE-2008-5015) [FIX]file: URIs inherit chrome privs if opened from chrome

Status: VERIFIED FIXED

(Core :: Security: CAPS, defect)

Description

[Daniel Veditz [:dveditz]]

the security alias received a report from Luke Bryan that file: URIs are given chrome privileges if opened in the same tab as a chrome (or privileged about:) page. This does not happen in the latest Firefox 2.0.0.17pre.

Steps:
 1. create a local file that contains the following script:
  <script>
    try{ alert((Components.classes) ? "Chrome -- bad!" : "Invalid test"); }
    catch (e) { alert( "Not chrome -- good!"); throw (e); } 
  <script>
 2. open about:config
 3. in the same tab open the file created in step 1.

The first step is to get a regression range. It would be ironic if it were my bug 230606 "restrict file: abilities" fix.

Comment 1

[Daniel Veditz [:dveditz]]

To exploit this you have to
 1. get attack code saved locally
 2. get a user to open a privileged about or chrome: URI
 3. convince the user to open the local file

It seems a tall order, but I wouldn't rule it out completely. Our MFSA 2008-35 advisory, for example, described a Safari+Firefox blended threat that accomplished 1 and 2 (now fixed).

Comment 2

[Martijn Wargers (dead)]

This regressed between 2008-05-28 and 2008-05-29:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-05-28+04&maxdate=2008-05-29+09&cvsroot=%2Fcvsroot
I think a regression from bug 433328.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

More likely a regression from bug 435362.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, a local backout confirms that. Does a docshell know if its current document is a chrome document? It seems like we shouldn't inherit for a file URI if our current document is chrome.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I thought we weren't supposed to inherit unless the URI we're linked from was itself a file:// URI.  Did this check get lost?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

It looks like that code was removed with the followup checkin for bug 402983.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Or rather it got moved into CheckMayLoad, but in this case we're not calling nsPrincipal::CheckMayLoad.

Comment 8

[Samuel Sidler (old account; do not CC)]

Dan, this needs an owner. I believe CAPS is you... Not going to block on it for now.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 10

[Daniel Veditz [:dveditz]]

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

r=dveditz

Comment 11

[Boris Zbarsky [:bzbarsky]]

Pushed changeset fddfa9210e76.

Comment 12

[Boris Zbarsky [:bzbarsky]]

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

We should take this on branch.

Comment 13

[Daniel Veditz [:dveditz]]

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

Approved for 1.9.0.3, a=dveditz for release-drivers

Comment 14

[Boris Zbarsky [:bzbarsky]]

Actually, this was changeset 0e630c354e2b.

Fixed on branch.

Comment 15

[Johnny Stenback (:jst)]

bz, should this be marked fixed?

Comment 16

[Boris Zbarsky [:bzbarsky]]

Uh, yes.  ;)

Comment 17

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

Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102304 GranParadiso/3.0.4pre.

Comment 18

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

Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081023 Minefield/3.1b2pre.