navigator.mozApps.install can be used to enumerate local file names

RESOLVED FIXED in Firefox 16

Status

Core Graveyard
DOM: Apps
RESOLVED FIXED
5 years ago
2 months ago

People

(Reporter: pauljt, Assigned: myk)

Tracking

({privacy, sec-high})

Trunk
mozilla18
privacy, sec-high
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-basecamp:+, firefox15 disabled, firefox16+ fixed, firefox17+ fixed, firefox18+ fixed, firefox-esr10 unaffected)

Details

(Whiteboard: [qa-][advisory-tracking-])

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

5 years ago
Calling navigator.mozApps.install with a file:// URI behaves differently depending on whether the file exists or not. A malicious webpage could use this enumerate local file names.

navigator.mozApps.install('file:///non-existant') throws a file not found error, where as if the file exists but can't be parsed, the onerror method of the domrequest is fired.

Its pretty slow though so a real world attack isn't too likely. There is a hacky PoC below which attempts to guess your username on a mac, but it probably only works if you have a three letter username ;)

http://untrusted.creativemisuse.com/testMozAppFileURI.html

Tested in 18.0a1 (2012-09-17)
Hmm...could this problem affect FF Android and B2G? Or will this only affect desktop?
this feels be higher than moderate -- if it can be speeded up you could spider someone's whole disk which might yield all kinds of juicy details. And there's probably some way to parallelize this.
Keywords: privacy, sec-high

Updated

5 years ago
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
tracking-firefox18: --- → ?
Bill and Myk - We should find an owner to fix this. This sounds bad.
(Assignee)

Comment 4

5 years ago
I suspect this does affect Android and B2G, although I haven't tested.  cc:ing a few more folks who can provide insight and/or a fix.
We should disallow installs for a manifest served by any protocol other than http or https on all platforms. The drawback of this is that it might make development a little harder as developers will have to setup a local server to test installs, but I think that's an acceptable tradeoff.
(In reply to Myk Melez [:myk] [@mykmelez] from comment #4)
> I suspect this does affect Android and B2G, although I haven't tested. 
> cc:ing a few more folks who can provide insight and/or a fix.

Okay. I'll put this on the nom list just in case for basecamp triage.
blocking-basecamp: --- → ?

Comment 7

5 years ago
It sounds like we'd like to fix this before FF16's release, and we may wontfix if a patch is ready later than Tuesday (beta 5's go to build) given the fact that this was internally found. Sending over to myk to help prioritize.
Assignee: nobody → myk
tracking-firefox16: ? → +
tracking-firefox17: ? → +
tracking-firefox18: ? → +
(Assignee)

Comment 8

5 years ago
Created attachment 663120 [details] [diff] [review]
patch v1: validate manifest/package URL scheme

Here's a patch that does what Anant suggests: restricts schemes from which apps can be installed to 'http' and 'https' by throwing an exception if the scheme is not one of those values.  The patch does this for both install() and installPackage().

Unfortunately, I don't know how to test installPackage(), neither automatically nor manually, but the patch includes mochitests for install().  I also ran other apps mochitests I know about (dom/tests/mochitest/webapps/, dom/apps/tests/, and extensions/cookie/test/), and they all passed.
Attachment #663120 - Flags: review?(fabrice)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Comment on attachment 663120 [details] [diff] [review]
patch v1: validate manifest/package URL scheme

Review of attachment 663120 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure if should throw an exception or fire an error on the DOMRequest, but let's land that and revisit that later if we need.
Attachment #663120 - Flags: review?(fabrice) → review+
(Assignee)

Comment 10

5 years ago
Created attachment 663141 [details] [diff] [review]
patch v1 for aurora branch
Attachment #663141 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 11

5 years ago
Created attachment 663142 [details] [diff] [review]
patch v1 for beta branch
Attachment #663142 - Flags: approval-mozilla-beta?
(Assignee)

Comment 12

5 years ago
dveditz: my proposed vague commit comment is:

  bug 791943 - validate install URL scheme; r=fabrice

Any suggestions on wording from a security perspective?  Also, is there a best practice for landing such patches on multiple branches (in terms of order in which patches are landed, timing of landings relative to spinning of builds, etc.)?
(Assignee)

Comment 13

5 years ago
Comment on attachment 663120 [details] [diff] [review]
patch v1: validate manifest/package URL scheme

The new "security bug approval process" doesn't go into effect until the end of the month, but the flag is already live, and perhaps setting it will get this bug on the radar of security folks who can provide input into how best to land its patches.

[Security approval request comment]
How easily can the security issue be deduced from the patch?
-> It seems fairly easy to figure out the problem from the patch.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
-> The code comments and my proposed vague commit comment are ok, but the tests are fairly inculpatory.

Which older supported branches are affected by this flaw?
-> I think they all are, including the current stable release (Firefox 15).

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
-> I have backports for Aurora and Beta.  I imagine that a backport for Release would be easy to cook up.

How likely is this patch to cause regressions; how much testing does it need?
-> This patch is unlikely to cause real regressions, although I could imagine it causing a failure in some apps test I'm not aware of.  I ran all the apps tests I'm aware of, but I haven't pushed it to TryServer yet because it seems insecure to do so.
Attachment #663120 - Flags: sec-approval?
(Assignee)

Comment 14

5 years ago
Created attachment 663201 [details] [diff] [review]
patch v1 for release branch
Attachment #663201 - Flags: approval-mozilla-release?
(In reply to Myk Melez [:myk] [@mykmelez] from comment #14)
> Created attachment 663201 [details] [diff] [review]
> patch v1 for release branch

? release-branch? Why the release branch?
(Assignee)

Comment 16

5 years ago
(In reply to Jason Smith [:jsmith] from comment #15)
> ? release-branch? Why the release branch?

The bug appears to exist in all branches, including the release branch.  Bug 772638 disabled the mozApps API in Firefox 15 for Desktop, so the bug doesn't affect that product.  But it isn't clear to me if it affects any other products and whether or not it's severe enough to warrant a hotfix release.

Nor am I the right person to make those determinations.  So I created the patch and requested approval to land it to get it on the radar of release and security drivers who can do so.

Updated

5 years ago
blocking-basecamp: ? → +
status-firefox-esr10: --- → unaffected
status-firefox15: --- → disabled
status-firefox16: --- → affected
status-firefox17: --- → affected
status-firefox18: --- → affected
Comment on attachment 663201 [details] [diff] [review]
patch v1 for release branch

We're 2 weeks from release and the feature is disabled in 15, so no need to land or chemspill.
Attachment #663201 - Flags: approval-mozilla-release? → approval-mozilla-release-
Comment on attachment 663120 [details] [diff] [review]
patch v1: validate manifest/package URL scheme

Good to land on m-c, in preparation for m-a and m-b.
Attachment #663120 - Flags: sec-approval? → sec-approval+
Comment on attachment 663141 [details] [diff] [review]
patch v1 for aurora branch

[Triage Comment]
Approving for Aurora 17 and Beta 16 given this is a low risk fix for a sec-critical bug (and we need to get this into beta 5 to fix in FF16).
Attachment #663141 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #663142 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/63643fbbde88
https://hg.mozilla.org/releases/mozilla-aurora/rev/4c6a240afef7
https://hg.mozilla.org/releases/mozilla-beta/rev/fa2b95b5030f

Please make sure that hg is configured to generate all the necessary metadata needed for checkin. It makes life easier for those checking in on your behalf.
https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in
status-firefox17: affected → fixed
Flags: in-testsuite+
Keywords: checkin-needed
status-firefox16: affected → fixed

Updated

5 years ago
Whiteboard: [qa-]
http://mozillamemes.tumblr.com/post/21637966463/yes-i-have-a-condition-that-forces-me-to-insert
"Land it on inbound first, they said. Make sure it sticks there before landing bustage on other branches, they said..." Well, that was fun. Backed out on all branches for mochitest-other failures.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2f1680ac8e
https://hg.mozilla.org/releases/mozilla-aurora/rev/a776a022281b
https://hg.mozilla.org/releases/mozilla-beta/rev/c71e253025fc

https://tbpl.mozilla.org/php/getParsedLog.php?id=15460874&tree=Mozilla-Beta

9461 INFO TEST-START | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul
9462 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | install returned Not enough arguments [mozIDOMApplicationRegistry.install]
9463 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | the error callback was called
9464 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | #MANIFEST_PARSE_ERROR# is expected to be true per template #== "MANIFEST_PARSE_ERROR"#
9465 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | the error callback was called
9466 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | #INVALID_MANIFEST# is expected to be true per template #== "INVALID_MANIFEST"#
9467 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | at least one notification displayed
9468 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | the success callback was called
9469 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | name is undefined as expected
9470 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | at least one notification displayed
9471 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | the success callback was called
9472 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | name is undefined as expected
9473 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | installPackage not in navigator.mozApps
9474 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | the error callback was called
9475 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | #DENIED# is expected to be true per template #== "DENIED"#
9476 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | navigator.mozApps.mgmt.addEventListener is not a function
9477 INFO TEST-KNOWN-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | navigator.mozApps.mgmt.removeEventListener is not a function
9478 INFO TEST-PASS | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | attempt to install nonexistent file: URL throws exception - INVALID_URL_SCHEME: 'file'; must be 'http' or 'https' should equal INVALID_URL_SCHEME: 'file'; must be 'http' or 'https'
NOTE: child process received `Goodbye', closing down
9479 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | Test timed out.
status-firefox16: fixed → affected
status-firefox17: fixed → affected
Flags: in-testsuite+
The failure on aurora and beta was the same. It was different on inbound, however.

8635 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | an unexpected uncaught JS exception reported through window.onerror - NS_NOINTERFACE: Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIFileURL.file] at chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul:159
(Assignee)

Comment 23

5 years ago
Created attachment 663998 [details] [diff] [review]
patch v2: test existent file: URL behavior using file:///

(In reply to Ryan VanderMeulen from comment #20)
> Please make sure that hg is configured to generate all the necessary
> metadata needed for checkin. It makes life easier for those checking in on
> your behalf.
> https://developer.mozilla.org/en-US/docs/
> Creating_a_patch_that_can_be_checked_in

I generate patches using Git, and I'm not sure how to embed that information into a patch generated by Git, but I'll look into it!

In any case, I almost always commit my own changes, and I would have done so in this case too, if hg.mozilla.org hadn't repeatedly aborted my attempts to pull the various branches on Friday evening in preparation for landing the changes.


(In reply to Ryan VanderMeulen from comment #21)
> http://mozillamemes.tumblr.com/post/21637966463/yes-i-have-a-condition-that-
> forces-me-to-insert

Yes, I agree, and normally I do test changes on TryServer.  I only didn't do so in this case because it seems insecure.  I'm certainly happy to entertain ideas for how to do so securely!

Nevertheless, I'm sorry you had to go to the trouble of committing and then backing out the changes. :-/


> 9479 ERROR TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/
> test_install_errors.xul | Test timed out.

(In reply to Ryan VanderMeulen from comment #22)
> The failure on aurora and beta was the same. It was different on inbound,
> however.
> 
> 8635 ERROR TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/
> test_install_errors.xul | an unexpected uncaught JS exception reported
> through window.onerror - NS_NOINTERFACE: Component returned failure code:
> 0x80004002 (NS_NOINTERFACE) [nsIFileURL.file] at
> chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/
> test_install_errors.xul:159

Hmm, I can't reproduce this problem, neither on central nor on beta (still building aurora), on the Windows machine I'm currently using.  And the same tests also worked when I ran them on my Mac on Thursday.  So I don't know why they failed on the test machines.

Nevertheless, I can remove the code in question while still testing the behavior of a file: URL to an existent file by using the file: URL file:///, which points to the existent root of the filesystem (it doesn't matter that the URL points to a directory; its existence is sufficient to cause the behavior of the API to differ from its behavior for a nonexistent file).

Here's a patch for central that tests file:///.
Attachment #663120 - Attachment is obsolete: true
Attachment #663998 - Flags: review?(fabrice)
(Assignee)

Comment 24

5 years ago
Created attachment 663999 [details] [diff] [review]
patch v2 for beta branch
Attachment #663142 - Attachment is obsolete: true
(Assignee)

Comment 25

5 years ago
Created attachment 664001 [details] [diff] [review]
patch v2 for aurora branch
Attachment #663141 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #663201 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 26

5 years ago
(In reply to Myk Melez [:myk] [@mykmelez] from comment #23)
> Hmm, I can't reproduce this problem, neither on central nor on beta (still
> building aurora), on the Windows machine I'm currently using.  And the same
> tests also worked when I ran them on my Mac on Thursday.  So I don't know
> why they failed on the test machines.

One possibility is that the tests are run against builds in which the JSMs have been packaged into the omnijar, and their resource: URLs are aliases for jar: URLs that don't implement nsIFileURL.file (but do still claim to implement nsIFileURL, for some reason).
(Assignee)

Comment 27

5 years ago
Since the change was already pushed to public branches, I pushed the new patch to TryServer:

https://tbpl.mozilla.org/?tree=Try&rev=b700cd94718c
FYI, there was a recent newsgroup discussion about running s-s bugs through Try, and the belief was that the signal to noise ratio is low enough to make watching it for hacking fodder unproductive.
(Assignee)

Comment 29

5 years ago
TryServer tests are looking good (modulo the usual orangefactors).
Attachment #663998 - Flags: review?(fabrice) → review+
(Assignee)

Updated

5 years ago
Attachment #663999 - Flags: approval-mozilla-beta?
(Assignee)

Updated

5 years ago
Attachment #664001 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 30

5 years ago
All builds/tests on the TryServer run came up green or orangefactor:

https://tbpl.mozilla.org/?tree=Try&rev=b700cd94718c
http://hg.mozilla.org/mozilla-central/rev/63643fbbde88
(Assignee)

Comment 32

5 years ago
lsblakk: umm, that's a link to yesterday's landing (comment 20), which was then backed out (comment 21).  Did you mean to provide a different link?
(In reply to Myk Melez [:myk] [@mykmelez] from comment #30)
> https://tbpl.mozilla.org/?tree=Try&rev=b700cd94718c

Green on Try (music to my ears).

https://hg.mozilla.org/integration/mozilla-inbound/rev/59877fe3f1e2
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/59877fe3f1e2
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox18: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla18

Updated

5 years ago
Attachment #663999 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

5 years ago
Attachment #664001 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 35

5 years ago
Comment on attachment 664001 [details] [diff] [review]
patch v2 for aurora branch

https://hg.mozilla.org/releases/mozilla-aurora/rev/229a0c3a8f95
Attachment #664001 - Flags: checkin+
(Assignee)

Comment 36

5 years ago
Comment on attachment 663999 [details] [diff] [review]
patch v2 for beta branch

https://hg.mozilla.org/releases/mozilla-beta/rev/ba26736aadf2
Attachment #663999 - Flags: checkin+
(Assignee)

Updated

5 years ago
status-firefox16: affected → fixed
status-firefox17: affected → fixed
Whiteboard: [qa-] → [qa-][advisory-tracking-]
Group: core-security

Updated

2 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.