proposal-dynamic-import icon indicating copy to clipboard operation
proposal-dynamic-import copied to clipboard

Does this proposal enable membrane penetration?

Open erights opened this issue 9 years ago • 23 comments

Raised by @allenwb at https://github.com/tc39/proposal-dynamic-import/issues/25

(And privately earlier by @bmeck)

Membrane penetration would likely be a show stopper.

Attn @fudco @dtribble @caridy @jfparadis

erights avatar Nov 16 '16 20:11 erights

Can you please explain?

domenic avatar Nov 16 '16 20:11 domenic

The whole point of a membrane is to provide an impenetrable boundary. From the beginning, Proxies and WeakMaps were designed to support impenetrable and (almost) transparent membranes.

erights avatar Nov 16 '16 20:11 erights

@erights He's asking for code I suppose.

Mouvedia avatar Nov 16 '16 20:11 Mouvedia

Note that an important difference between static import and dynamic import is that with static import (and its module specifier) is detectable by scanning source code. With dynamic import this is not the case. And it's not just the dynamic specifier that could be a problem. The dynamic import operation can be dynamically constructed and then executed using eval.

allenwb avatar Nov 16 '16 20:11 allenwb

Dynamic import is detectable by scanning source code; that is the point of using the syntactic form.

domenic avatar Nov 16 '16 20:11 domenic

Dynamic import is detectable by scanning source code; that is the point of using the syntactic form.

not if eval is available

allenwb avatar Nov 16 '16 20:11 allenwb

I'm happy to ban import() in eval.

domenic avatar Nov 16 '16 20:11 domenic

@domenic all forms of eval?

Mouvedia avatar Nov 16 '16 20:11 Mouvedia

On second thought that has drawbacks that outweigh the considerations here. It should be up to the sandboxing environment to modify eval() to perform whatever restrictions it wants.

domenic avatar Nov 16 '16 20:11 domenic

Maybe eval could have a second argument allowImport?

EDIT: On a second thought, this would require the argument to be inherited for all nested evals; I don't know if that's possible.

mik-jozef avatar Feb 20 '17 12:02 mik-jozef

The problem with dynamic import is that it's tricky to determine exact dependencies statically. We must re-think the whole idea, and identify two completely separate activities:

  1. Dependency declaration. Can help to a browser to download a module, and to a bundler to include the module in the bundle (or even lazily refer to it, if it's dynamic import).

  2. Dependency linking. The moment when all code starts working together.

Imagine the old alternative CommonJS world, where module name may be constructed dynamically in a very complex way that not Browserify, neither webpack will be able to detect the dependency.

If developers had a hint to a ESM loader or a bundler, which modules it may import - both statically, and dynamically - then bundling and loading may become 100% deterministic without any heuristics.

Considering the ESM-only world, dynamic import is the same as require() by its virtue. It introduces indeterminism.

I think the whole Allen's idea of static imports is completely incompatible with dynamic imports (obviously).

I'm a huge fan of CommonJS (but, please, don't fight with me about it), but I really want to find the rationale behind ESM. I'm looking how <script type="module"> can help us to solve the problem of cache busting and race condition-free large website updates (and refreshes of only changed modules).

I think that new mechanisms are yet to be invented and emerged, and ESM in browsers is not production ready. Perhaps, we must wait again, about 2 to 3 years, before new standards supposed to solve the dynamic import problem will be implemented in all major browsers.

Poor developers, we must wait again... :-(

avesus avatar Dec 23 '17 21:12 avesus

Is this proposal on hold due to this? Is there anything that could be done to help?

rhuanjl avatar May 26 '18 17:05 rhuanjl

Is this proposal on hold due to this?

Hi @rhuanjl , yes. The security hole created by the import expression is real. It rolled out in browsers before I knew it had. As a result, we had a critical security flaw in SES. Fortunately, this was responsibly disclosed to Google (I was still at Google at the time) so we could fix it before it was maliciously exploited. To fix it, we had to take the incredibly painful and expensive step of making parsing mandatory rather than optional, because currently there's no other way to reliably intercept this.

The current Frozen Realms shim, which Salesforce will be using in production as part of the Locker service, will have the same problem and currently will have to implement the same expensive solution. An additional cost of using a user-written parser is that the parser cannot be kept in sync with the grammar accepted by the platform it is running on. It adds a further delay to the availability of new safe syntactic features that our users should have been able to use.

In a way this is my fault. When I agreed that the import expression could proceed to stage 3, it was with the understanding that the upcoming Realms proposal would provide ways to intercept them (the import and import.meta expressions), so that systems like SES and the Locker service to use Realms to stay safe. Long term, this is still a good plan. However, I did not think through the horrible consequences of these things being deployed before Realms. The import expression and import.meta expression must not become standard before there's a standard way to intercept them, so that a JavaScript implementation that omits them can still be standards conformant. Alternatively, if they were moved to Annex B as normative-optional, then an implementation could still omit them while staying conformant.

Is there anything that could be done to help?

The Realms proposal that just advanced to stage 2 does not yet have traps for the import and import.meta expressions. However, it already protects against them, in that it will unconditionally throw when evaluating code that contains these expressions. Effectively, it implements the sublanguage in which these constructs are absent, which can currently still be considered a language conforming to the ES standard. Whether Realms grows the traps or not, once it advances and gets deployed, so that systems like SES and Locker can use Realms to remain safe, we can unblock the import and import.meta expressions.

erights avatar May 26 '18 18:05 erights

@erights thank you for the detailed explanation - really informative.

rhuanjl avatar May 26 '18 18:05 rhuanjl

@domenic is there any information somewhere about CSP and dynamic import? Maybe there is a middle ground here that we can explorer in the interim.

caridy avatar May 29 '18 17:05 caridy

Dynamic import is treated like any other script load with CSP, so indeed CSP suffices to secure code against dynamic import.

domenic avatar May 29 '18 17:05 domenic

@domenic isn't what @erights intends for frozen realms the answer to the question: what does ES need to support reflective implementation of CSP (or equivalent policy enforcers)

allenwb avatar May 29 '18 21:05 allenwb

Would the Realms proposal provide a way to implement this security boundary? The transform trap can reliably hit all eval calls, and certain versions of that proposal either provided for disallowing import or trapping its calls.

The import expression and import.meta expression must not become standard before there's a standard way to intercept them, so that a JavaScript implementation that omits them can still be standards conformant. Alternatively, if they were moved to Annex B as normative-optional, then an implementation could still omit them while staying conformant.

I'm not sure what it would really mean to leave these things out of the standard, or put them in Annex B. In practice, they are already shipped in multiple browsers, based on the specification in this repository. Is the purpose of the standard to describe a hypothetical "secure" JavaScript, or to be the coordinating mechanism used to advance actual implementations? If we leave these things out of the main specification, it just means a more confusing and disorganized state of the real-world specification, not any more security for anyone.

littledan avatar Jul 16 '18 16:07 littledan

hypothetical "secure" JavaScript

SES, ADsafe and FBJS come to mind.

Mouvedia avatar Jul 16 '18 16:07 Mouvedia

@Mouvedia I don't mean to call those systems hypothetical. I'm talking about the JavaScript language which they run on top of. In practice, they are often running on top of the versions of JavaScript shipped in browsers, which often includes this proposal.

littledan avatar Jul 16 '18 17:07 littledan

@domenic, just for the sake of curiosity, why was the import() function not specified as a value property of the global object?

If that was the case, library implementers could remove the global, reimplement it, or shadow it inside membranes, which would probably solve the concerns expressed so far.

jfparadis avatar Jul 25 '18 21:07 jfparadis

https://github.com/tc39/proposal-dynamic-import#an-actual-function

domenic avatar Jul 25 '18 21:07 domenic

Maybe I'm just being dense here but I don't see the difference between loading a static module and loading a dynamic module. Modules and their exports are implicitly shared: if I import {x} from 'y', I get the same module exports object and static state regardless of where I import it from. If a Realm is intended to be isolated then an "import" in that Realm would need to solve the problem of shared module state. The dynamic import doesn't change that problem and any mechanism to solve it would need to solve one, should solve it for both.

Yes, dynamic import can act as an eval and that has security implications and security tools need to consider this and it is "new" and thus creates holes until they are updated but this aspect at least isn't a fundamental problem.

concavelenz avatar Jan 24 '19 18:01 concavelenz