1475831 - WebExtension: content scripts should be injected in data URL pages

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[kkapsner]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180705213349

Steps to reproduce:

When a webExtension has content scripts with should be injected in "<all_urls>" they should also be injected in data URL pages (data:text/html;...) that come from a web page (e.g. over an iFrame).

The same issue is known to Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=55084
And the implications for privacy addons (e.g. fingerprinting protection) are the same. A page can easily circumvent such addons by using an iFrame with a data-URL.

Reproduction scenario.
- Install the example webExtension https://github.com/mdn/webextensions-examples/tree/master/notify-link-clicks-i18n
- go to https://itty.bitty.site/#/data:text/html;base64,PGh0bWw+PGJvZHk+PGEgaHJlZj0iaHR0cHM6Ly9leGFtcGxlLm9yZyI+Y2xpY2s8L2E+PC9ib2R5PjwvaHRtbD4=
- accept the disclaimer
- click on "click"


Actual results:

The example.org page loads without a notification


Expected results:

The webExtension should display a notification

Comment 1

[Kris Maglione [:kmag]]

We only match data: URLs if match_about_blank is true and the content script matches the URL of the document's actual origin. This is intentional.

Comment 2

[kkapsner]

If you modify the example webExtension to have match_about_blank to be true (I also added all_frames to be sure) it still does not work.

  "content_scripts": [
    {
      "match_about_blank": true,
      "all_frames": true,
      "matches": ["<all_urls>"],
      "js": ["content-script.js"]
    }
  ],

And adding the content script via browser.tab.executeScript just prompts that the host permission is missing.

Also you can see in the two screenshots I added that the content script is injected in about:blank but not in the data-URL.

Comment 3

[kkapsner]

Attachment: about:blank script injection

Comment 4

[kkapsner]

Attachment: data-URL none script injection

Comment 5

[kkapsner]

Where is this behaviour of match_about_blank on data-URLs documented?

Comment 6

[kkapsner]

Here is the real world issue with this bug: https://github.com/kkapsner/CanvasBlocker/issues/208

Comment 7

[Rob Wu [:robwu]]

This is a regression from bug 1324406.

In bug 1324406, data:-URIs started having unique origins, hence the match_about_blank does not apply any more.

If we want to support data:-URIs in content scripts, there are multiple directions:
- Match data:-URIs when match_about_blank is set.
- Introduce match_data_url (https://crbug.com/55084#c25 ).
- Support "data:" as a match pattern. Our MatchPattern class currently already supports "data:" [1], and <all_urls> includes data:-URIs by default. However, content scripts do not accept "data:" as an explicit match pattern [3], and the WebExtensionContentScript::Matches logic does not account for data:-URIs with a unique principal [4].


[1]: https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/toolkit/components/extensions/MatchPattern.cpp#266
[2]: https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/toolkit/components/extensions/MatchPattern.cpp#266,289,293-294
[3]: https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/toolkit/components/extensions/schemas/manifest.json#469
[4]: https://searchfox.org/mozilla-central/rev/943a6cf31a96eb439db8f241ab4df25c14003bb8/toolkit/components/extensions/WebExtensionPolicy.cpp#472,484-486,501,507

Comment 8

[Rob Wu [:robwu]]

Attachment: datauri.zip

Steps to reproduce:
1. Load attached extension via about:debugging (or unzip it and do: web-ext run).
   (The extension opens example.com, loads a data:-URI)
3. Look at the bottom of the frame and the bottom of the page.

Expected result:
- "This is injected by a content script at a data:-URL" in the frame.
- "This is injected by a content script at a https:-URL" in top-level document.

Actual result:
- (frame not modified)
- "This is injected by a content script at a https:-URL" in top-level document.


Additionally:
- Visit about:config and set security.data_uri.unique_opaque_origin to false, and then you will see the expected behavior.

Comment 9

[ubbiurro]

bug 1417975 notes that data: URIs work from iframe src, but not from location bar, object tag, or frameset frame tags

Comment 10

[kkapsner]

In my tests it also does not work in iframes. I think something was changed there in the meantime.

Comment 11

[kkapsner]

Any news here?

Comment 12

[Rob Wu [:robwu]]

I may take a look at this bug when I strart working on a related issue with sandboxed documents (bug 1411641).

Comment 13

[kkapsner]

@Rob Wu: What issue about sandboxed documents did you want to link? You linked this bug and not the other one.

Comment 14

[Rob Wu [:robwu]]

(In reply to kkapsner from comment #13)

@Rob Wu: What issue about sandboxed documents did you want to link? You linked this bug and not the other one.

I edited my comment. I meant to link to bug 1411641.

Comment 15

[Rob Wu [:robwu]]

Chrome is going to support data:-URLs with the match_data_urls field, with similar semantics as match_about_blank.
https://bugs.chromium.org/p/chromium/issues/detail?id=55084#c45

Comment 16

[Rob Wu [:robwu]]

match_data_urls is not going to be implemented after all; instead we have designed a new property with more sane semantics, called match_origin_as_fallback. This allows extensions to run content scripts in data:, blob:, about:, solely based on the origin of the document (and in case of opaque origins, the "precursor" origin that created the document).

The discussion of the design of this new property starts at https://bugs.chromium.org/p/chromium/issues/detail?id=55084#c60 and is summarized in https://bugs.chromium.org/p/chromium/issues/detail?id=55084#c70 .

Comment 17

[kkapsner]

I hope this will also be available in browser.contentScripts.register.

Comment 18

[Giorgio Maone [:ma1]]

Any updates?

Comment 19

[Rob Wu [:robwu]]

This is on my list, but I need to finish other tasks before I can get to this.

Comment 20

[kkapsner]

Any updates?

Comment 21

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

The severity field for this bug is relatively low, S3. However, the bug has 13 votes.
:robwu, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 22

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 24

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

Set release status flags based on info from the regressing bug 1324406

Comment 25

[gfyocqr987]

[redacted - mhoye]

Comment 26

[gfyocqr987]

[redacted - mhoye]

Comment 27

[gfyocqr987]

[redacted - mhoye]

Comment 28

[kkapsner]

Any progress on this topic?

Comment 29

[Rob Wu [:robwu]]

This is likely going to be resolved when bug 1411641 gets addressed, as part of the "matchOriginAsFallback" / match_origin_as_fallback flag.

Comment 30

[Rob Wu [:robwu]]

Attachment: Bug 1475831 - Add test coverage for match_about_blank (blob, javascript, data schemes)

Add missing test cases for existing match_about_blank behavior:

• javascript:-URLs resolve to about:blank and code should inject there.

The following cases are not documented, but this patch adds test
coverage before we change the implementation:

• blob:-URLs are currently matched because they inherit the principal.

• data:-URLs. They are special because they have an opaque origin now,
  but for a very long time they did not. Additionally, there is an
  undocumented case where data:-URLs can execute.

Comment 31

[Rob Wu [:robwu]]

Attachment: Bug 1475831 - Drop redundant check in URLInfo::InheritsPrincipal

URLInfo::InheritsPrincipal() is used to support the match_about_blank
feature. When that flag is used, a content script should execute in
about:blank and about:srcdoc.

When the logic was introduced in Firefox 55 (bug 1368102), data:-URLs
could inherit the principal like about:blank / about:srcdoc, thanks to
this URI_INHERITS_SECURITY_CONTEXT check in URLInfo::InheritsPrincipal.
In Firefox 57 (bug 1324406), data:-URLs got an opaque origin by default.
In Firefox 83 (bug 1552168), this became the only supported behavior
with the removal of the security.data_uri.unique_opaque_origin pref.

Because the URI_INHERITS_SECURITY_CONTEXT check is not doing anything,
this patch removes it.

The patch does not add data: support to match_about_blank; the intent is
to add that later as part of match_origin_as_fallback.

Comment 32

[Rob Wu [:robwu]]

Attachment: Bug 1475831 - Add match_origin_as_fallback support

Adds match_origin_as_fallback support to content_scripts declared
statically in manifest.json.

Support for this option in dynamic registration and execution will be
handled in the next patches.

Comment 33

[Rob Wu [:robwu]]

Attachment: Bug 1475831 - Support opaque origins in scripting.executeScript etc.

Add support for dynamic insertion of content scripts and styles in
sandboxed documents / frame with an opaque origin, via the scripting API
(executeScript / insertCSS / removeCSS).

Previously, we used the matchAboutBlank flag internally, which matches
less than the matchOriginAsFallback option that was just introduced.
Specifically, sandboxed about:blank/about:srcdoc and data:-URLs were
not scriptable before, but now they are.

To encourage the use of the scripting API, the MV2-only versions in the
tabs API did not get this feature.

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1eabe598552b
Add test coverage for match_about_blank (blob, javascript, data schemes) r=rpl
https://hg.mozilla.org/integration/autoland/rev/82a799bad541
Drop redundant check in URLInfo::InheritsPrincipal r=rpl
https://hg.mozilla.org/integration/autoland/rev/8a60644a680e
Add match_origin_as_fallback support r=rpl
https://hg.mozilla.org/integration/autoland/rev/aed79cb14fe2
Support opaque origins in scripting.executeScript etc. r=willdurand

Comment 35

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1eabe598552b
https://hg.mozilla.org/mozilla-central/rev/82a799bad541
https://hg.mozilla.org/mozilla-central/rev/8a60644a680e
https://hg.mozilla.org/mozilla-central/rev/aed79cb14fe2

Comment 36

[Ryan VanderMeulen [:RyanVM]]

Should we call this out in the Fx128 relnotes? Please nominate if yes.

Comment 37

[Rob Wu [:robwu]]

No need for it to be part of the relnotes. It should be mentioned in the Firefox 128 for Developers article on MDN, which is covered by the dev-doc-needed keyword.

Comment 38

[rbloor]

Change are ready for review:

• MDN Docs for content script injection into data URL pages #34491
• BCD Support for match_origin_as_fallback in content_scripts manifest key #23553