Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: fetch event in service worker can bypass the limitation of extension's CSP #418

Open
hanguokai opened this issue Jul 10, 2023 · 11 comments
Labels
discussion Needs further discussion security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. topic: csp Related to content security policy enforcement topic: remote code Related to remote code execution topic: service worker Related to service worker background scripts

Comments

@hanguokai
Copy link
Member

Overview

In Manifest V3, all of your extension's logic must be part of the extension package. You can no longer load and execute remotely hosted files.1

Default CSP is below 2, and disallows remote code.

  "content_security_policy": {
    "extension_pages": "script-src 'self'; object-src 'self';"
  }

The problem is that fetch events can easily bypass this limitation at the moment. I reported this issue at https://crbug.com/1463420

How to achieve it

// in service worker
async function getRemoteCode(url) {
  let resp = await fetch(url);
  let text = await resp.text();
  return new Response(text);
}

self.addEventListener("fetch", async (event) => {
  const url = new URL(event.request.url);

  // generate dynamic code or load remote code
  if (url.pathname == '/remote') {
    const codeUrl = url.searchParams.get("url");
    const codeText = url.searchParams.get("text");
    if (codeUrl) {
      event.respondWith(getRemoteCode(codeUrl));
    } else if (codeText) {
      event.respondWith(new Response(codeText));
    }
  }
});

Then, in extension pages, you can load any remote code or dynamic code like below.

function loadScript(url) {
  let script = document.createElement('script');
  script.onload = function () {
    alert('load success');
  };
  script.onerror = function () {
    alert('load fail');
  };
  script.src = url;
  document.head.appendChild(script);
}

function loadRemoteCode() {
  let codeUrl = "https://code.jquery.com/jquery-3.7.0.min.js";
  let url = `/remote?url=${encodeURIComponent(codeUrl)}`;
  loadScript(url);
}

function runDynamicCode() {
  let code = "console.log('hello')";
  let url = `/remote?text=${encodeURIComponent(code)}`;
  loadScript(url);
}

What does it affect?

In extension pages, it can:

  1. generate dynamic code
  2. load remote code or load a local copy of the remote code.

Note: This approach works fine if you open an extension page or refresh the page (Ctrl+R). But if you force a refresh (Ctrl+Shift+R), then this approach will go wrong, because at this point the browser ignores the fetch event and forces a lookup for the file.

What does it not affect?

Depending on browsers' implementation (I only tested it in Chrome), it does not affect:

  1. scripting.executeScript()
  2. scripting.registerContentScripts() or static content scripts in manifest.json
  3. resources in "web_accessible_resources"

Is this a serious security issue?

Due to the fact that Manifest V2 is not completely prohibited and there are still a large amount of Manifest V2 extensions, this is not a serious security issue at present.

But since prohibiting loading remote code is one of security features in Manifest V3, this is indeed a security issue.

This is only an Extension issue, not a Web issue.

Web Extension (MV3)
Allow loading remote code by default Loading remote code is not allowed by default
Both CSP and service worker are controlled by the site owners themselves, they can do whatever they want. CSP is enforced by the browser, but service worker is written by developers, they are not the same entity.
script-src 'self' means that the code(resource) can only come from the same origin (not file) The intention of the browser is that the code can only come from local files in the extension package.

On the Web, a resource is represented by a URL, and its response may or may not be from a file on the server. Files are not used to represent resources. CSP 'self' restricts the origin of a resource rather than restricting it from a file. So this is not a Web issue, resources via service worker do come from the same origin.

How to fix this issue? (possible solutions)

Solution 1: Check where the code comes from

Any code must come from a file or memory cache, don't allow from service worker.

Solution 2: Forbid fetch event in service worker

fetch event is a core function for Websites or Web app, but it is not necessary for extensions. Although it is a little useful, not many people use it in extensions. It is not indispensable, and there is no fetch event in Manifest V2.

Footnotes

  1. Improving security in Manifest V3

  2. Manifest - Content Security Policy

@hanguokai hanguokai added the security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. label Jul 10, 2023
@ianwill93
Copy link

Just wanted to drop a comment that solution 2 sounds a bit crazy. Unless there's an alternative & modern way to access APIs--the only permitted way to run dynamic code--how would that be viable?

@dotproto
Copy link
Member

dotproto commented Jul 10, 2023

I'll have to defer to @oliverdunk as to Chrome's current stance on this topic, but I can share some background based on my time with the team.

To my knowledge Manifest V3's design never sought to completely prevent extensions from executing remote code. Instead, the platform changes introduced in Manifest V3 were aimed at removing the most common methods of executing arbitrary code. That's why when I discussed this publicly I tried to set expectations that Chrome would be "enforcing the remotely hosted code (RHC) restrictions through a combination of platform and policy changes" (chromium-extensions post).

Due to the inherently online nature of the web and the powers inherent in extensions, I don't think it's possible to completely prevent an extension from fetching and executing remote code. In the future Fenced Frames may provide useful concepts that the extension platform can build on to limit an extension's network access, but we'll have to see how that proposal evolves.

TL:DR; my current view is that this is a known limitation of the extension platform.

@erosman
Copy link

erosman commented Jul 11, 2023

Note: The comment is mainly regarding the issue of remote code in general.

In practical terms, it is not possible to completely remove the risk of remote code inclusion (there are other methods besides fetch).

Please note that background service-worker/event-page fetch and XMLHttpRequest (not available in Chrome SW) are core necessities for userscript managers, as well as many other extensions.

There are many legitimate uses, e.g. getting data, that is only possible from a background process not affected by the (often abusive) webpage CSP. There are also legitimate remote code use cases (e.g. including APIs such as maps, captcha, etc).

While it is completely understandable that some extensions may abuse the capabilities given to the extensions, all extensions should not be tarred with the same brush. I have seen far more webpages that abuse users' privacy, security, and browsing experience, in comparison to extensions. Reducing the capability of an extension to a level lower than a webpage would greatly affect user experience.

It is worth considering that ...

  • Content scripts are subject to the webpage's CSP, while background scripts are not
  • Should an uncontrolled webpage be able to control and dictate extensions' behaviour?

@tophf
Copy link

tophf commented Jul 11, 2023

This is an old exploit, which shows how arbitrary and flimsy the restriction is, and it was already admitted as WontFix by @dotproto 😮 in https://crbug.com/1239976.

@tophf
Copy link

tophf commented Jul 11, 2023

Please note that background service-worker/event-page fetch and XMLHttpRequest (not available in Chrome SW) are core necessities for userscript managers, as well as many other extensions.

The issue is about a different thing: the fetch event and not the fetch function.
Nevertheless, the fetch event may be also useful to extensions.

@tophf
Copy link

tophf commented Jul 11, 2023

there is no fetch event in Manifest V2

ManifestV2 extensions can also register a service worker to make use of the event.

@oliverdunk
Copy link
Member

Thanks for the writeup @hanguokai - I really appreciate it.

In this case, I think @dotproto's thoughts line up pretty well with what I would have said. In short, we're trying to make doing the right thing easy, and the wrong thing hard. We know there are still ways to execute remotely hosted code (as mentioned, there are more beyond this) and in those cases we rely on developers to be aware of what they're doing and our policy to enforce against any extensions that abuse this power. We'd rather do that then lock things down too much and potentially break more legitimate use cases than we need to.

I'll leave the Chromium bug open so the team can reply there but I think we can probably close this one, since this seems like mostly a Chrome discussion?

@hanguokai
Copy link
Member Author

This is an old exploit, which shows how arbitrary and flimsy the restriction is, and it was already admitted as WontFix by @dotproto in https://crbug.com/1239976.

I know this may have been reported by others, but I didn't find it in the chromium bug list before, probably using the wrong keywords. I first thought of this issue 2 years ago, and I thought of it again last week in response to another question.

We're enforcing the remotely hosted code (RHC) restrictions through a combination of platform and policy changes.

Now this issue becomes another question. Is the prohibition of using RHC strictly guaranteed by technical means, or is it guaranteed by technical + policy means? Because of the restrictions enforced in the CSP, I previously thought this was guaranteed by technical means. And I think this loophole can be closed by technical means. If it is guaranteed by policy, it will be very unreliable.

ManifestV2 extensions can also register a service worker

As far as I remember, this is just to facilitate developers migrating to MV3, which was only supported a few years ago. It's not how MV2 was originally used.

I'll leave the Chromium bug open so the team can reply there but I think we can probably close this one, since this seems like mostly a Chrome discussion?

I don't think this is a Chrome-only issue, as other browsers will also support MV3 including this restriction of RHC.

@oliverdunk
Copy link
Member

Is the prohibition of using RHC strictly guaranteed by technical means, or is it guaranteed by technical + policy means? Because of the restrictions enforced in the CSP, I previously thought this was guaranteed by technical means. And I think this loophole can be closed by technical means. If it is guaranteed by policy, it will be very unreliable.

It is guaranteed only by policy. While I agree that we could close this specific loophole, the problem is that there are many other loopholes which are harder to fix. As an example we want to allow extensions to run code in the main world and inevitably in the main world there are ways of doing whatever the site can. In reality I think trying to add more protections here would harm honest developers the most, by making reasonable things harder, while bad actors would always be able to find workarounds that we cannot prevent purely from a technical POV and need policy to complement.

I don't think this is a Chrome-only issue, as other browsers will also support MV3 including this restriction of RHC.

I'll leave this open for now so the group can make a decision 🙂

@hanguokai
Copy link
Member Author

My viewpoint is:

  • This issue only affect extension context. user-scripts and some other ways are executed in web context (not extension context).
  • Developers rarely use "fetch event" in extensions. It is designed for Web, not for extensions. I also don't think fixing this specific issue will harm honest developers.
  • If all browsers believe that technical means should be abandoned in favor of policy means for this specific issue, then we can close this issue.

@hanguokai hanguokai added topic: csp Related to content security policy enforcement topic: remote code Related to remote code execution topic: service worker Related to service worker background scripts labels Jul 11, 2023
@zombie
Copy link
Collaborator

zombie commented Jul 20, 2023

In our current (prototype) implementation of extension service workers in Firefox, we don't support fetch events, and have no plans to change that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Needs further discussion security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. topic: csp Related to content security policy enforcement topic: remote code Related to remote code execution topic: service worker Related to service worker background scripts
Projects
None yet
Development

No branches or pull requests

7 participants