Closed Bug 1587336 Opened 5 years ago Closed 3 years ago

Consider disallowing data: URLs for dynamic imports in content-scripts

Categories

(WebExtensions :: General, enhancement, P2)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: freddy, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: sec-want)

A lot of extensions are using eval, which is a bad idea. We block this through a CSP, except in content-scripts.
AFAIU we feel uncomfortable blocking eval in content scripts, due to the prevalence of eval in existing add-ons and widely used frontend frameworks.

But import('data:text/javascript,'+sourcecode) is equivalent to eval and much less popular. So I'd like to explore the idea of disallowing dynamic import of data URLs in content-scripts before there is adoption.

The idea is to add a check in the implementation of import such that we check the current global (e.g., whether we are in a content-script) and throw for resources starting with 'data'.

I'm filing this as a security bug, not to alert malicious add-on authors. But there's no immediate risk here (as reflected in the sec-other security rating).

TIL we don't actually support dynamic imports. See https://bugzilla.mozilla.org/show_bug.cgi?id=1536094.
Setting this one here as a blocker, so we make a decision before we implement.

Blocks: 1536094

A lot of extensions are using eval, which is a bad idea. We block this through a CSP, except in content-scripts.

FYI there is work in progress to block eval by default in content scripts as well, per bug 1323630 .

If we support dynamic imports in content scripts, we're likely not going to support data:-URLs (only files from the extension package).

Priority: -- → P2

I don't see any work in progress on bug 1323630 (conversation ended 2 years ago). However, with Bug 1581608 for manifest v3, eval might be (probably be) blocked by default. Extensions using eval would have to specify a csp to allow that in the manifest, making them easy to identify.

Since the feature is not implemented yet, the concern expressed here is not a security bug.

I do believe that when we ship dynamic imports in content scripts, that we should only allow URLs with the moz-extension scheme, which excludes data:-URLs.

Agree with Rob, it looks like Freddy filed as a security bug before realizing this is not implemented yet, so I'll open it up.

Group: firefox-core-security

I do believe that when we ship dynamic imports in content scripts, that we should only allow URLs with the moz-extension scheme, which excludes data:-URLs.

This seems like a valid approach to me. This would also remove some concerns about WebExtension content-script importing modules used by the webpage and running into potential global/compartment mismatches.

The patch in bug 1536094 disallows loading non moz-extension URLs.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.