1487353 - Extensions can run content scripts in local files and read any other local file

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Rob Wu [:robwu]]

Attachment: permission-warning.png

When an extension requests the "<all_urls>" or "file://*/*" permission, the permission warning at install time is: "Access your data for all websites".

(good:) Despite the permission, extensions cannot read local files from the background page (bug 1266960). This is also reflected in the isAllowedFileSchemeAccess returning false (bug 1270306).

(bad:) However, when the user opens a local file in Firefox, extensions can successfully inject content scripts in the local file page. That content script can then read any other file in the file system, even though the security.fileuri.strict_origin_policy preference is set to true.


When I open a local file in a browser, I do not expect extensions that can "access your data for all websites" to have access to any other local file on my system.

I believe that we should block local file access by default, and optionally add a preference to allow users to allow some extensions access to local files.


[ In Chromium, extensions cannot interact with local files unless the user opts in at the extension overview page. This offers the best security (of local files), at the expense of usability (extensions that legitimately need local file access have to ask the user to manually opt in). ]

Comment 1

[Rob Wu [:robwu]]

Attachment: file-perm.xpi

STR:
1. Install the extension
  * To see the permission warning:
    - visit about:config to set xpinstall.signatures.required to false (requires Nightly or unbranded build).
    - install the attached xpi file
  * To follow the test without checking the warning (any Firefox build):
    - Download the attached extension and load it at about:debugging

2. Open a local file in the browser, e.g. file:///tmp
3. Look at the global browser console.
4. Look at the console of the tab.

Expected:
Extension has should not have access to files:
- After step 2, the page should look normal.
- After step 3, the console should display Security Error: Content at moz-extension://.../_generated_background_page.html may not load or link to file:///..."
- After step 4, the tab's console should not contain anything.

Actual:
- After step 2, the page is read
- After step 3, the global console contains the expected message.
- After step 4, the tab's console contains the content of a local file (e.g. /etc/passwd).

Comment 2

[Rob Wu [:robwu]]

I brought this up during yesterday's engineering meeting, and some asserted that the ability to run scripts in local files is expected behavior.

Mike, what is your product opinion on this?
Does "Access your data for all websites" include "all local files" too?

Comment 3

[Kris Maglione [:kmag]]

This is the expected behavior.

Comment 4

[David Durst [:ddurst]]

Expected by us, but we decided to file this bug for Product to weigh in on that expectation as well.

Comment 5

[Mike Conca [:mconca]]

I'm on-board with this being expected behavior, after all, loading a local file isn't much different (to the browser) from one delivered by a server.  Content scripts probably should be injected in both cases.

But I need clarification around this statement in comment 0 - "That content script [injected into a locally loaded file] can then read any other file in the file system". Does that mean that if I load a local file, an injected content script now has access to every other file in my local filesystem?  For example, could a malicious extension convince a user to open a local file (any would do) and then proceed to try and open and read /etc/passwd?

If true, this seems like a huge backdoor security hole, which makes me think I am not understanding this correctly, since :dveditz marked it as sec-moderate.

Comment 6

[Kris Maglione [:kmag]]

(In reply to Mike Conca [:mconca] (Denver, CO, USA UTC-6) from comment #5)
> But I need clarification around this statement in comment 0 - "That content
> script [injected into a locally loaded file] can then read any other file in
> the file system". Does that mean that if I load a local file, an injected
> content script now has access to every other file in my local filesystem? 
> For example, could a malicious extension convince a user to open a local
> file (any would do) and then proceed to try and open and read /etc/passwd?

No. Content loaded from one file: URL only has access to files in the same directory and subdirectories thereof. If you loaded a file from your home directory, scripts running in it (whether extension scripts or not) could access any other file under your home directory, but not any file above it.

There is a preference which controls this behavior, though, and it can be made more lenient. But users who do that assume the risks.

Comment 7

[Kris Maglione [:kmag]]

(In reply to Mike Conca [:mconca] (Denver, CO, USA UTC-6) from comment #5) 
> For example, could a malicious extension convince a user to open a local
> file (any would do) and then proceed to try and open and read /etc/passwd?

Also, /etc/passwd is not super important, which is why it's generally world-readable. /etc/shadow generally *is* super important, which is why it is not world-readable.

Comment 8

[Rob Wu [:robwu]]

(In reply to Mike Conca [:mconca] (Denver, CO, USA UTC-6) from comment #5)
> I'm on-board with this being expected behavior, after all, loading a local
> file isn't much different (to the browser) from one delivered by a server. 
> Content scripts probably should be injected in both cases.

I agree that this is desirable behavior for files that are explicitly opened in the browser by the user.

But when a local file is opened, that file does not only get access to that local file, but also any other local file in the same directory or a subdirectory. This even happens if the preference bug that I mentioned is fixed.

> But I need clarification around this statement in comment 0 - "That content
> script [injected into a locally loaded file] can then read any other file in
> the file system". Does that mean that if I load a local file, an injected
> content script now has access to every other file in my local filesystem? 
> For example, could a malicious extension convince a user to open a local
> file (any would do) and then proceed to try and open and read /etc/passwd?

Yes, that is the case. Currently content script access to a local file = content script access to all local files (that are readable by Firefox).
In my attached example, I visited a page in file:///tmp/, and read file:///etc/passwd.


(In reply to Kris Maglione [:kmag] from comment #6)
> No. Content loaded from one file: URL only has access to files in the same
> directory and subdirectories thereof. If you loaded a file from your home
> directory, scripts running in it (whether extension scripts or not) could
> access any other file under your home directory, but not any file above it.
> 
> There is a preference which controls this behavior, though, and it can be
> made more lenient. But users who do that assume the risks.

You're thinking of the security.fileuri.strict_origin_policy preference.
In my report I mentioned that this pref was set to true (the default).
Yet the content script was still able to read /etc/passwd.

( When I ran the same XMLHttpRequest/fetch snippet from the devtools console in the page's context, the request was blocked as expected. )


(In reply to Kris Maglione [:kmag] from comment #7)
> Also, /etc/passwd is not super important, which is why it's generally
> world-readable. /etc/shadow generally *is* super important, which is why it
> is not world-readable.

The exact file does not matter; to demonstrate the severity it suffices to show that I have access to a file outside the directory tree of the local file.
/etc/passwd being world-readable is why it was chosen as the PoC (for Linux; on other platforms i used different locations).

Comment 9

[Mike Conca [:mconca]]

Comment 8 makes me think this should be a sec-high bug.  Opening a local file should not give an extensions access to the entire local file system.  And to be honest, I'm uncomfortable with extensions even being able to read all files in the directory and all subdirectories of a local file.

Comment 10

[Daniel Veditz [:dveditz]]

The latter is an old problem: we were the first browser to break the "all file:/// are same origin" historical behavior and went about as far as we could at the time. Since then some other browsers (chromium/webkit) have leap-frogged that and gone to "each file is a unique origin". We have an item on the DOM-security roadmap to follow-suit: bug 1500453

As to severity, because users have to install Web Extensions most problems get rated a step down because that's a hurdle to exploitation. sec-high generally means all or most users are vulnerable to a remote attack. This is limited to users to have installed the extension, and even then it's a local attack (or a local component of a social engineering attack that involves convincing a user to download something, which itself is a mitigation to the severity).

From comment 8
> In my attached example, I visited a page in file:///tmp/, and read file:///etc/passwd.

In comment 1 you said "Open a local file in the browser, e.g. file:///tmp". Under "sibling files and subdirectories" I would expect a file /tmp to be able to see /etc/passwd. I would not expect /tmp/foo to be able to read /etc/passwd (though I do know of a bug where that could be the result under certain circumstances).

I don't think this needs to be a P1 because I doubt there's anything WebExtensions can fix until bug 1500453 fixes the underlying origin problem.

Comment 11

[Daniel Veditz [:dveditz]]

The real problem here in my mind is that the permission prompt says "for all websites" and the user may be completely surprised that includes local files that are not websites. Maybe file:/// urls should require a different explicit permission that causes additional text about files in the prompt. That kind of fix would not depend on the origin changes in bug 1500453

Comment 12

[Rob Wu [:robwu]]

(In reply to Daniel Veditz [:dveditz] from comment #11)
> The real problem here in my mind is that the permission prompt says "for all
> websites" and the user may be completely surprised that includes local files
> that are not websites. Maybe file:/// urls should require a different
> explicit permission that causes additional text about files in the prompt.
> That kind of fix would not depend on the origin changes in bug 1500453

Agreed, we should have an explicit warning for file access, separate from "all websites".

(In reply to Daniel Veditz [:dveditz] from comment #10)
> From comment 8
> > In my attached example, I visited a page in file:///tmp/, and read file:///etc/passwd.
> 
> In comment 1 you said "Open a local file in the browser, e.g. file:///tmp".
> Under "sibling files and subdirectories" I would expect a file /tmp to be
> able to see /etc/passwd. I would not expect /tmp/foo to be able to read
> /etc/passwd (though I do know of a bug where that could be the result under
> certain circumstances).

When I open file:///tmp/d/e/e/p/, I can still read /etc/passwd with the PoC extension (whereas running the same fetch call from the console in the context of the tab fails as expected).

With the browser content toolbox, I see that the origin is definitely not a sibling of file:///etc/:
tabs[0].content.document.nodePrincipal.origin
// result: "file:///tmp/d/e/e/p/"
// (and "file://UNIVERSAL_FILE_URI_ORIGIN" if security.fileuri.strict_origin_policy is set to true.)


I did some more debugging and found why the two cases behave differently:
- From a moz-extension:-page, file:-loads are blocked by nsScriptSecurityManager::CheckLoadURIWithPrincipal because the scheme of the page is not "file" (but moz-extension).
  This is also visible in the console as:
  "Security Error: Content at moz-extension://87473217-286f-457d-a940-0cae2da171f3/manifest.json may not load or link to file:///etc/passwd."

- From a content script at file://, the content script has an expanded principal that includes the page (a file: scheme). This causes the nsScriptSecurityManager::CheckLoadURIWithPrincipal check to pass.
- Then we continue at BasePrincipal::AddonAllowsLoad, which uses WebExtensionPolicy::CanAccessURI.
  Since the extension has <all_urls> permissions, which includes the "file" scheme, the load is allowed.
- Then the request continues without issues.


The minimal reproduction steps to see the above is:
1. Visit file:///tmp/ (just to have easy access to a file principal).
2. Open browser content toolbox.
3. Run this snippet:
{
  // Principal of built-in Screenshots add-on, which has <all_urls> permission.
  let extprinc = Cc["@mozilla.org/webextensions/extension-process-script;1"].getService().wrappedJSObject.getExtensionChild('[e-mail]').principal;
  // Principal of file://-URL of the tab that you just opened.
  let fileprinc = Cu.getObjectPrincipal(tabs[0].content);

  // Error: TypeError: NetworkError when attempting to fetch resource.
  Cu.Sandbox([fileprinc], {wantGlobalProperties:["fetch"]}).eval(`fetch("file:///etc/passwd")`);
  // Error: TypeError: NetworkError when attempting to fetch resource.
  Cu.Sandbox([extprinc], {wantGlobalProperties:["fetch"]}).eval(`fetch("file:///etc/passwd")`);
  // Succeeded (=not seeing error)
  Cu.Sandbox([fileprinc, extprinc], {wantGlobalProperties:["fetch"]}).eval(`fetch("file:///etc/passwd")`);
}

The error with just the file principal is good and expected.
The error with the extension principal is bug 1266960.
The succeeding request of the file+extension principal is this bug's test case (comment 1).

Bug 1266960 can be solved by modifying nsScriptSecurityManager::CheckLoadURIWithPrincipal to always allow requests from extensions to continue (so that all access is controlled through extension permissions).
If we solve bug 1266960, then the behavior from this bug is expected.

If we don't want to fix bug 1266960 (i.e. we basically want to ignore the "file:" permission...), then this bug can be resolved by ignoring the file:-permission at WebExtensionPolicy::CanAccessURI. The problem of ignoring file:-permission at that layer is that the method is also used to determine whether a content script is allowed to be injected.

In either case, I believe that the file:-permission should not be granted automatically, but be opt-in via a permission warning (or via about:config).

Comment 13

[Shane Caraveo (:mixedpuppy)]

We could have a preference for allowing local file access like Chrome does.  Disallow any kind of access at all unless the user checks that, throw up ugly warnings when they do so.  It would break some current extensions, not sure if there are chrome compat issues.  I've never particularly cared for any file access at all without very explicit permission.

Comment 14

[Daniel Veditz [:dveditz]]

When I open file:///tmp/d/e/e/p/, I can still read /etc/passwd with the PoC extension (whereas running the same fetch call from the console in the context of the tab fails as expected).

We have now fixed bug 1500435, but given the above description the new checks will be bypassed as much as the original ones were.

Comment 15

[Rob Wu [:robwu]]

I can confirm that this bug still happens on the latest Nightly.

This bug is the reason that I do generally not load untrusted add-ons in Firefox. It is reasonable for extensions to be able to access the content of a local file in a browser, but not to be able to read any other file on the local filesystem (e.g. from ~/.ssh/) via a content script in an unrelated local file.

Comment 16

[Luca Greco [:rpl] [:luca] [:lgreco]]

This bug concerns me too, I think that opening a local file url is not something that regular users do that often, but developers may be much more likely to do it and this would allow an extension content script to read any of their local files (include sensible ones like .npmrc and .ssh/* files).

I agree with the last comment from Shane (Comment 13) that in the long run we should implement a user-controlled "file access" permission as we did for the "private browsing access" (and I would personally prefer to not fix Bug 1266960 until we have done that), but it is likely going to not be a small project (it may be a bit smaller than the "private browsing access" one, but still more than just a few changes).

And so in the meantime (at least in my opinion) it would be good to fix this one (it has been open for a while now) by making the content script fetch/XHR requests to behave exactly like the "file://"-url webpage that the content script is wrapping (and eventually re-evaluate if we should allow the content scripts to access files from any localition only after we have implemented a user-controller "file access" permission).

ExpandedPrincipal::MayLoadInternal method (which is currently returning true if any of the principal is allowed to load the url) seems to be a reasonable place to apply a fix for this issue.

My proposal is to leave the webpage principal to decide if the target file url should be accessible by the content script (which would make the content script to behave like the "file://"-url webpage when the target url is a file url), e.g. by doing something like the following:

bool ExpandedPrincipal::MayLoadInternal(nsIURI* uri) {
  bool isFile = uri && uri->SchemeIs("file");

   for (uint32_t i = 0; i < mPrincipals.Length(); ++i) {
     auto principal = BasePrincipal::Cast(mPrincipals[i]);

    if (isFile && principal->AddonPolicy()) {
      // If the uri has a file scheme and the ExpandedPrincipal is
      // from an extension content script, leave the outcome to be
      // consistent with the webpage principal.
      continue;
    }

    if (principal->MayLoadInternal(uri)) {
       return true;
     }
   }
}

We could then revisit (or revert) this change once we have implemented the user-controller "file access" permission.

Comment 17

[Shane Caraveo (:mixedpuppy)]

I can no longer reproduce this using the str provided in comment 1. I get the "expected" outcome.

Comment 18

[Rob Wu [:robwu]]

Can still reproduce with str from comment 1 on Linux in Nightly 72.0a1 buildID 20191028094851 .

Note that even at the time of reporting, the expected result happens in the background page.
You need to look at the console of the tab to see that the content script unexpectedly has access to file:-URLs.

Comment 19

[Shane Caraveo (:mixedpuppy)]

Correction on my results from the str in comment 1 for osx, results repeated here:

Expected:
Extension has should not have access to files:

• After step 2, the page should look normal.
• After step 3, the console should display Security Error: Content at moz-extension://.../_generated_background_page.html may not load or link to file:///..."
• After step 4, the tab's console should not contain anything.

Actual:

• After step 2, the page is read
• After step 3, the console should display Security Error: Content at moz-extension://.../_generated_background_page.html may not load or link to file:///..."
• After step 4, the tab's console should not contain anything.

Step3/4 are as I expect.

Step 2, the content script is injected, but I would expect that since the extension has file permissions and I opened the file myself.

Rob, why should the file look normal at step 2?

Comment 20

[Shane Caraveo (:mixedpuppy)]

The entire STR is still reproducible linux.

Comment 21

[Rob Wu [:robwu]]

Should have been:

Expected:
After step 2, the page should look normal.
Actual:
After step 2, the page is red (edited: not "read")

This is because the original report also asserted that extensions should not be able to run scripts in file:-URLs, unless explicit approval was given. Later (e.g. comment 5), this was deemed an intended feature, and the scope of the report was reduced to only cover the part where a content script could read any other file.

The most accurate expectations are now:

Expected:

• After step 2, the page is red.
• After step 3, the console should display Security Error: Content at moz-extension://.../_generated_background_page.html may not load or link to file:///..."
• After step 4, the tab's console should not contain anything.

Actual:

• After step 2, the page is red
• After step 3, the global console contains the expected message.
• After step 4, the tab's console contains the content of a local file (e.g. /etc/passwd).

Comment 22

[Rob Wu [:robwu]]

Attachment: Bug 1487353 - Test for file requests from content scripts

Comment 23

[Rob Wu [:robwu]]

Patch for this bug was attached in https://bugzilla.mozilla.org/show_bug.cgi?id=1420296#c60 . The test case here should land in the future, when the bugfix has landed.

Comment 24

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:robwu, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 25

[Rob Wu [:robwu]]

It's a regression test for a security bug. Will land later.

Comment 26

[Shane Caraveo (:mixedpuppy)]

Rob, status on this?

Comment 27

[Rob Wu [:robwu]]

This bug has been fixed in bug 1420296 (in Firefox 74, not on ESR68). The only patch here is a unit test, which I intend to land together with the unit test from the other patch (https://phabricator.services.mozilla.com/D54543).

To avoid drawing too much attention to the tests (which are functional exploits), I want to hold off with landing this at least until 74 reaches release. Maybe even until the next ESR.

Comment 28

[Rob Wu [:robwu]]

(updating bug flags; fixed in 74 (shipping today); will land unit test and close bug)

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Test for file requests from content scripts:
https://hg.mozilla.org/integration/autoland/rev/69a2553a6c864503207056b8784d7a03c06b6303
https://hg.mozilla.org/mozilla-central/rev/69a2553a6c86