Last Comment Bug 479943 - (CVE-2009-1839) Restrictions on file-URL-to-file-URL scripting don't appear to be working properly
(CVE-2009-1839)
: Restrictions on file-URL-to-file-URL scripting don't appear to be working pro...
Status: RESOLVED FIXED
[sg:moderate]
: fixed1.9.1, verified1.9.0.11
Product: Core
Classification: Components
Component: Security: CAPS (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on: 424484
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-24 01:49 PST by Adam Barth
Modified: 2009-06-11 15:15 PDT (History)
10 users (show)
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part one of test case (11 bytes, text/html)
2009-02-24 01:50 PST, Adam Barth
no flags Details
part two of test case (117 bytes, text/html)
2009-02-24 01:50 PST, Adam Barth
no flags Details
Proposed fix (2.49 KB, patch)
2009-03-27 17:37 PDT, Boris Zbarsky [:bz]
dveditz: review+
jst: superreview+
jst: approval1.9.1+
dveditz: approval1.9.0.11+
Details | Diff | Review

Description Adam Barth 2009-02-24 01:49:25 PST
Collin and I were experimenting with the restrictions on file-URL-to-file-URL scripting and found some strange behavior.  Imagine you have the following files on your local file system:

/foo/bar/target.html (contains some text)
/foo/bar/child/index.html (contains an iframe to /foo/bar/target.html)

Steps to reproduce:

1) Type "file:///foo/bar/child/index.html" in the location bar and hit enter.
2) Notice that /foo/bar/child/index.html cannot access /foo/bar/target.html
3) Type "file:///foo/bar/target.html" in the location bar and hit enter.
4) Type "file:///foo/bar/child/index.html" in the location bar and hit enter.
5) Notice that /foo/bar/child/index.html *can* access /foo/bar/target.html

Bizarre, no?
Comment 1 Adam Barth 2009-02-24 01:50:10 PST
Created attachment 363851 [details]
part one of test case
Comment 2 Adam Barth 2009-02-24 01:50:30 PST
Created attachment 363852 [details]
part two of test case
Comment 3 Brandon Sterne (:bsterne) 2009-02-24 09:17:08 PST
I'm seeing different behavior than in your STR:

> 1) Type "file:///foo/bar/child/index.html" in the location bar and hit enter.
> 2) Notice that /foo/bar/child/index.html cannot access /foo/bar/target.html

I have /home/brandon/Desktop/child/index.html and
/home/brandon/Desktop/target.html

and when I load child/index.html the contents of target.html are visible in the iframe.  I cleared my cache before loading the child page.

> 3) Type "file:///foo/bar/target.html" in the location bar and hit enter.
> 4) Type "file:///foo/bar/child/index.html" in the location bar and hit enter.
> 5) Notice that /foo/bar/child/index.html *can* access /foo/bar/target.html

It could see target.html before, but I can see why that would be strange if you weren't seeing the parent in step 1.  Can someone else confirm the behavior?
Comment 4 Adam Barth 2009-02-24 09:46:14 PST
> and when I load child/index.html the contents of target.html are visible in the
> iframe.

Sorry, my description wasn't clear.  Try clicking the button that says "Go!".  The first time through, the outer page can read the contents of the inner page (its HTML shows up in the alert) but the second time through it can't.
Comment 5 Daniel Veditz [:dveditz] 2009-02-24 12:55:04 PST
> Bizarre, no?

Bizarre yes, but as intended for better or worse.

The first efforts at tightening up the same-origin policy ("same directory only") broke too many real-world use cases (e.g. on-disk reference material). Allowing parents to reach into sub-directories but not vice-versa created asymmetric permission checks which isn't a good idea in our codebase (bug 402983) and still managed to break a few real-world cases that have wide distribution such as Sun's Java documentation. Making it blindly symmetric meant a file could open a guessed root document (C:\boot.ini on windows?), and then inject script into that document that could spider down elsewhere making all the checks moot.

The current behavior is that files are only same-origin only if they have exactly the same principal, but when a file opens another file in the same directory or a subdirectory it bestows it's own principal on that file. If a file opens a file in a parent or sibling directory it gets its own principal, and the two cannot access each others content. This does lead to the odd order dependency you noted, where if the child is opened by the parent they access each other but if the parent is opened by the child they can't.

When opening a file the new file's location is compared against the principal of the opener and not the actual location (again, in an attempt to not break real-world documents). So if you have
    docs/parent.html
    docs/chap1/section1-1.html
    docs/chap2/section2-1.html
then if parent.html opens section1-1.html, either parent or section1-1 can open section2-1 and all three can interact.

In no case are files allowed to read the contents of a file: URI that is a directory listing, even a subdirectory listing, to prevent spidering.

I'm unhiding this bug because this behavior should be documented, and as written probably INVALID. However it is quirky enough that there might be unintended misbehaviors worth investigating.
Comment 6 Boris Zbarsky [:bz] 2009-02-24 13:15:58 PST
I would think that the principal shouldn't inherit across loads via the url bar.  This bug says that it does, right?
Comment 7 Daniel Veditz [:dveditz] 2009-02-24 13:28:58 PST
rehiding. Files opened via the URL bar should get a fresh start. Is that what Adam is saying?
Comment 8 Adam Barth 2009-02-24 14:27:51 PST
(In reply to comment #7)
> rehiding. Files opened via the URL bar should get a fresh start. Is that what
> Adam is saying?

Yes.
Comment 9 Boris Zbarsky [:bz] 2009-02-24 15:13:56 PST
Yeah, so nsDocShell::InternalLoad will use the current document's principal as the owner if no owner is provided and a file:, data:, or javascript: URI is being loaded, and DoURILoad will use the owner if loading a file URI if the owner passes the CheckMayLoad() check.

Is there a reason we're getting an owner in InternalLoad in the file case?  I thought location sets and link clicks always passed in an owner directly, no?
Comment 10 Boris Zbarsky [:bz] 2009-03-27 17:37:17 PDT
Created attachment 369787 [details] [diff] [review]
Proposed fix

With this change we actually do pass the principal in for link clicks and form submission.  Subframe loads and location sets were already doing it.  This lets us remove the "inherit the owner" flag from this case, and to stop inheriting the owner for file:// URIs in all cases.

I did check that <a href="someOtherFile.html" target="_blank"> works to give the new page the same principal as the old one if the new page is in the same dir or a subdir.  We really need file:// mochitests.  :(
Comment 11 Daniel Veditz [:dveditz] 2009-04-11 09:48:27 PDT
Comment on attachment 369787 [details] [diff] [review]
Proposed fix

r=dveditz

I was waiting for a chance to test this, but for now I'll take bz's assurance that link navigation preserves the correct owner.
Comment 12 Daniel Veditz [:dveditz] 2009-04-11 10:13:52 PDT
Tested and it works as I'd expect.
Comment 13 Boris Zbarsky [:bz] 2009-04-13 08:35:47 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/354f5ac7485d

Tests will have to wait on bug 424484.
Comment 14 Boris Zbarsky [:bz] 2009-04-13 08:36:55 PDT
Comment on attachment 369787 [details] [diff] [review]
Proposed fix

We should take this on branches too.  This should be reasonably safe, I hope, but some baking is definitely needed.  The fact that we have no automated tests for this set of codepaths (nor a test infrastructure that could run them) makes things a bit worse than they should be.
Comment 15 Daniel Veditz [:dveditz] 2009-04-13 14:22:56 PDT
Not blocking for now, please renom after baking. We'll look at the approval request anyway.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2009-04-14 09:40:58 PDT
Comment on attachment 369787 [details] [diff] [review]
Proposed fix

Looks worth taking for 1.9.1.
Comment 17 Boris Zbarsky [:bz] 2009-04-15 17:04:02 PDT
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/38b8a7bc6130
Comment 18 Daniel Veditz [:dveditz] 2009-04-17 10:26:35 PDT
Comment on attachment 369787 [details] [diff] [review]
Proposed fix

Approved for 1.9.0.10, a=dveditz for release-drivers
Comment 19 Boris Zbarsky [:bz] 2009-04-23 06:01:47 PDT
Checked into CVS.
Comment 20 Al Billings [:abillings] 2009-05-11 16:52:56 PDT
I notice that I can no longer get the alert when I load the child directory case in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.11pre) Gecko/2009051111 GranParadiso/3.0.11pre. I also see:

JavaScript error: file:///Users/abillings/Desktop/child/index.html, line 1: Permission denied to get property Window.document

Is this really enough to validate this fix?
Comment 21 Boris Zbarsky [:bz] 2009-05-11 19:29:51 PDT
Sounds like correct behavior, yes. Compare to the behavior before this patch landed.
Comment 22 Al Billings [:abillings] 2009-05-14 16:22:35 PDT
I did compare the behavior. That's why I know about the alert. :-)

Following the steps in comment 0 with Firefox 3.0.10 and you will get the alert on the final step. Doing the same with 3.0.11, you do not get the alert.

Marking verified for 1.9.0.11.
Comment 23 Daniel Veditz [:dveditz] 2009-05-29 10:36:17 PDT
Irrelevant to the 1.8 branch where we let any file:/// document open any other file:/// document.

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