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 concerns with cross-origin structured cloned Modules #956

Closed
mtrofin opened this issue Jan 13, 2017 · 34 comments
Closed

Security concerns with cross-origin structured cloned Modules #956

mtrofin opened this issue Jan 13, 2017 · 34 comments
Milestone

Comments

@mtrofin
Copy link
Contributor

mtrofin commented Jan 13, 2017

At the moment, the spec allows for cross origin structured cloning of Modules.

The summary of the concern is: even assuming the semantic of structured cloning where we send the wire bytes under the hood and recompile them on the other side, an attacker could exploit a compiler bug this way, without the receiver having any chance of reacting. In contrast, if the receiver just got the wire bytes, they could make a decision, based on their origin, of dropping them or compiling them.

Should we disallow cross-origin structured cloning?

@lukewagner
Copy link
Member

That's an interesting point. I suppose this is true for all structured clone deserialization code; the only new thing here is the compiler is being added to this surface area. I suppose one mitigation is that, depending on the browser impl, the deserialization may only occur if the recipient has registered a cross-origin onmessage handler.

That being said, actually sharing the underlying machine code could have some huge advantages. A persistent question since the beginning has been how to share code so that you don't have, e.g., 100 copies of the Python runtime. (See #761 for an example.) A satisfying answer has been: well, with some coordination, you could have one origin (say, python.org) cache the runtime in one IDBObjectStore, and then all the users would postMessage python.org requesting the runtime which python.org would pull out of IDB and postMessage back. Then you have on-disk and even in-memory code sharing. There is the security concern, but then again, if you receive a Module from some other origin and go ahead and instantiate and run it, then you're already really trusting that origin...

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 13, 2017

The compiler being added to the surface area is a pretty big "only new thing" :)

To your last sentence - the difference between receiving some bytes, compiling them, and then instantiate + running them is that the transition data->code is in my control. In the structured cloning case, it's in the attacker's control.

@jfbastien
Copy link
Member

Unless we expose something like SRI, where the first-party domain can say "here's the hash of the .wasm file, I trust myCDN.example.com to have a matching binary".

@lukewagner
Copy link
Member

@mtrofin Yes, I appreciate that difference. Actually, pulling the two points together, though: if the structured clone impl always hands over machine code, then we're never allowing the attacker to execute the compiler.

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 13, 2017

But the machine code could be the attack vector.

@jfbastien
Copy link
Member

@mtrofin if the first-party says "yes, I absolutely trust this code, please run it" then whether it's an attack vector or not, and where it's loaded from, is immaterial: the situation is equivalent to the first-party site embedding the .wasm file on its own domain and serving from that domain.

@esprehn
Copy link

esprehn commented Jan 13, 2017

Why isn't that level of caching of ex. the python runtime an implementation detail of the caching strategy around the network stack, for example how we cache the generated code for scripts that come from the same URL? I'd prefer we didn't allow structured cloning even for same origin, having the compiled output of the same input be cached is something the engine can handle.

@lukewagner
Copy link
Member

@mtrofin Well, it's an attack vector, even without any browser-level bug, if it is being served by a malicious origin and you're running it in your origin; that's XSS.

@esprehn Implicit caches have a lot of limitations; see the motivations for ServiceWorkers. We actually have an implicit cache for asm.js machine code in FF and I'm quite eager to get away from it.

@ghost
Copy link

ghost commented Jan 14, 2017

I don't yet follow all the concerns raised, but it might not just be a matter of "embedding the .wasm file on its own domain" because it is not expected that wasm files will be deployed rather customer compressed files that would stream in and be translated to the actual wasm that would be compiled, so might there be an issue trusting the decompression/translation code too. In other words the provenance of the Module might not be represented by the wasm URL.

@jeisinger
Copy link

My assumption is that the postMessage proposal is to enable a site to send a compiled binary to another origin. This is different from sending the wasm file, as the target origin then would compile the wasm binary (which ensures that the resulting binary runs within the constraints of the VM). If the process running the sending site was compromised, it can send an arbitrary binary to the other origin which then would attempt to execute it without prior verification steps.

I'm not concerned about sending wasm files around, as they'll need to be compiled first anyways

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 14, 2017

I think the problem is more nuanced.

In the postMessage scenario, today (without wasm), the receiver can be given a string that it then evals. An attacking sender sends malicious JS - which can pass all validations of the VM. The defense mechanism is the receiver identifying the origin of that message, and only after that eval-ing.

In the wasm world, the recipient receives a Module object. This in itself does not mean any code of that Module was executed - the receiver has to first create an Instance. The receiver can still check the origin of the message before going these steps. So far, it all looks like the case before. However, the problem is that executable code has been laid out in memory, by the messaging pipeline, before the recepient had a chance to vet the origin of the message. So there is a period of time when, even with a judicious recepient, an attacker could have executable code injected this way, acting like landing pad for a subsequent attack. This time period starts with the message being received and ends with the GC collecting the code.

This is no better if we automatically compile the bytes upon receipt (this is the fallback mechanism of deserialization, if the serialized bytes don't work - e.g. cross VM versions), because malicious code may still be thus laid out in memory.

The key is giving the receiver the chance to vet the origin, and only then create executable code. This means, with our current APIs, delaying populating executable pages until Instantiate, if the Module came through serialization.

@jeisinger
Copy link

I'd hope we don't put the binary into executable memory before we actually intend to execute it, so this isn't different from posting a string.

If you eval a string you still have the guarantee that it runs within the boundaries of the VM. If you just receive some binary and then execute it without further validation you don't have that guarantee, and verifying the origin doesn't help with that either.

@jfbastien
Copy link
Member

I think this discussion is losing sight of the end-goal: example.com wants to load framework.wasm from cdn.com: everyone else loads framework.wasm from cnd.com and it's likely to already be downloaded and compiled. The compiled part is kinda different from the rest of the web.

We want to make this possible. Whether it be through IDB/postMessage/structured-clone, or a baked-in browser cache, or something else.

@lukewagner's proposal tries to put that power in the web developer's hands. @esprehn asks why it's not just implicitly done by the browser as a QoI thing.

@jeisinger
Copy link

We should make that possible using the regular mechanisms that the platform offers: I'd expect some kind of <script> tag or that loads a wasm file. The web developer can then control the caching via a service worker, and the platform can transparently cache the compiled version.

@ghost
Copy link

ghost commented Jan 14, 2017

@mtrofin Your concern about the memory layout appears to have solutions using memory protection which could delay access to the memory until 'accepted'.

@jfbastien @jeisinger But what if a lot of resources are pulled to support a framework, and what if these dependencies (of the wasm Module cached in the web browser) are not clear.

Does a service worker have an origin and can that be the point of trust. Can the service worker check all of its sources from the cdn with a hash etc and probe the web browser features and then generate a Module that is cached, and can code in other origins then just trust the origin of the service worker?

Or do we need a dataflow pipeline that can be trusted to generate the wasm Module and that includes the sources (and hashes) which could be bundled with the Module to allow code using it later to check that it trusts all these sources?

@flagxor
Copy link
Member

flagxor commented Jan 14, 2017

Unless I'm mistaken, this issue has ended up mixing together a security discussion with an API one. If that's the case, I'd like to disentangle the two concerns. (Please confirm one way or the other so I can split the issue.)

@mtrofin and I had originally thought the concerns @jeisinger raised elsewhere were about the safety of the structured cloning mechanism (and as a part of that when we were materializing executable code in chrome's implementation). We should avoid materializing pages containing executable code before the receiver has a chance to decide if it want to opt into instantiating the module (and a change to do this in chrome is in progress).

Beyond this, the security argument as to why this is ok cross-origin is that once a receiver instantiates a module received from another origin, they've opted out of origin isolation (in the same way a receiver that evals strings from another origin has). If the sender comes from an origin that has been compromised to the point it can forge messages on the channel, process isolation is broken. Process isolation cannot protect a receiver willing to execute code (via eval or wasm), as whatever method used to break process isolation in the sender can be posted to the receiver.

As for the API concern, I agree that being able to leverage service workers + fetch + possibly a script tags + maybe es6 modules might be useful in the future, but several things are missing there or will be challenging to standardize in a timely fashion including:

  • The Cache API currently assumes cached values are Response objects. It would need to be expanded to support compile code (to allow explicit control of when compilation occurs). It would also need to handle the distinct property that compiled code can become invalidated by browser upgrades.
  • Caching API implementations assume <10MB data, wasm modules can be >40MB + code
  • Wasm is intended to have feature parity with asm.js (so it can replace it). Script tags don't provide enough flexibility to subsume these use cases currently.
  • The es6 modules don't currently encompass use cases such as dynamically decoded modules. As the modules feature is intended to improve ergonomics, it also may not be flexible enough to replace asm.js use cases.
  • The Fetch API (which is where Cache API gets Response objects from), has the interesting note: Formats you would not want a network layer to be dependent upon, such as HTML, will likely not be exposed here. Rather, an HTML parser API might accept a stream in due course. This would seem to suggest adding wasm here might be inappropriate.
  • Safari currently does not support service workers (I believe?), whereas IndexedDB support is broad.

I believe all of the above are surmountable given sufficient time and we should do this in the near future. However, providing the low level mechanism of structured cloning gives developers sufficient control to produce the desired behavior now. Also, I don't believe having it is an architectural blemish, as it provides the flexibility to do interesting things we might not want to build into those higher level mechanisms by giving us a way to decide when and where to compile / store wasm code.

@esprehn @jeisinger with the understanding that rationalizing wasm modules with Caching API / Service Workers / ES6 modules / script tags is a longer term goal, does supporting structured clone as well concern you?

@jeisinger
Copy link

My security concern is not properly summarized as "it's like eval" because eval() still enforces the VM constraints. Running arbitrary assembly code does not. So unless structed clone would recompile from the wasm sources, my concern still stands.

I also don't think we should cut corners here to support potential use cases ASAP - this will just make it more difficult to reach the long term goals, because ppl have already build around a different API.

@ghost
Copy link

ghost commented Jan 15, 2017

@jeisinger If the attach were exploiting a systematic bug in a compiler then it would appear to make no difference if this were exploited in the sending origin to compromise the compiled wasm module or compromised in the receiving origin, so I guess you are worried about an attach that would compromise the compiler so that only the sender could exploit it by compromising the compiler whereas the receiver might not have a compromised compiler and thus not be vulnerable? Wouldn't it be enough of a concern if the compiler could be compromised in any origin? How practical would it be to define a separate context to compile the modules - then the input resources would be known and the compiled module would be reproducible from these inputs, and there would be no asymmetry between the sender compiling the module or the receiver.

@jfbastien
Copy link
Member

@jeisinger

My security concern is not properly summarized as "it's like eval" because eval() still enforces the VM constraints. Running arbitrary assembly code does not. So unless structed clone would recompile from the wasm sources, my concern still stands.

wasm compiled code can't do anything on its own, it needs imports and exports to touch the VM. It's pretty similar to a self-contained closure with inputs and outputs, and cannot access global state.

That being said, I agree we shouldn't cut corners. I simply don't understand the corner you're referring to.

@jeisinger
Copy link

@Wllang Yes, if the bug is in the compiler itself, it doesn't make a difference whether the result of the compilation is used or the wasm source. However, if we blindly execute the binary, any other exploit that allows for constructing a malicious postMessage payload can be turned into a code execution bug.

@jfbastien at the point where we receive the postMessage, there is no guarantee that it's the output of wasm compiled code. It's just an arbitrary binary.

My comment about cutting corners was wrt the argument that we should pick an inferior solution now instead of working towards a better solution just because of the time it takes.

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 15, 2017

The key is "if you execute the binary".

From what I understand (and paraphrasing others on this thread), today's security model (in a JS-only world) is that the receiver is responsible for checking the origin before attempting to do anything with a payload (including eval; in wasm, "eval" becomes "instantiate and execute the module"). If the receiver does not do that, all bets are off. Is that an accurate description?

I don't believe we're arguing about the secure receiver in the wasm case, please correct me if I've misunderstood that.

I think @jeisinger is arguing that, in the insecure receiver case (receiver that doesn't check sender), the wasm case is worse somehow. The counter argument is that "the security model is that all bets are off in that case already". I believe that's why @flagxor's eval parallel is accurate: while eval applies VM constraints, the eval-ed string can fish out username+passwords just fine.

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 15, 2017

Oh, forgot to ask: I can't see how eval() enforcing VM constraints matter. If the receiver blindly accepts and evals any string, I can send a JS script that fishes out usernames + passwords. The fact that wasm is compiled bytes offers a different attack vector, but the security mechanism guarding against that are the same: just don't eval-it (==instantiate + run it).

@jeisinger
Copy link

So maybe I'm missing something. Let me ask some clarifying questions:

Does "instantiate" mean that the compiler gets to look at the entire thing? I.e. in V8, will Turbofan get to see the data before it runs?

What is the payload in the post message? Is it something in wasm format, or something in say arm64 binary?

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 15, 2017

In V8, the payload of the message is a buffer containing a mix of arm64 and data, as produced by the code serializer. The serializer, among other things, de-contextualizes the code from the sender. When received, this payload is deserialized by the code deserializer and contextualized to the receiver. This currently happens as part of the structured cloning pipeline. The compiler is not involved in this, but the VM is, with the deserializer.

On the structured cloning pipeline receiving end, the user is given a reference to a WebAssembly.Module. The WebAssembly.Module isn't yet callable, because the code has no reference to a memory nor to foreign functions. Binding it to these things is what "instantiation" does. The result of instantiation is a WebAssembly.Instance, which exposes one or more callable functions (called "exports").

Example:

function receiveMessage(event)
{
if (!good_origin(event)) return;

var received_module = event.data;
var i = new WebAssembly.Instance(received_module, {fct1: a_function, fct2: another_function, memory: a_byte_buffer});
i.exports.export1(1, 2, 3);
}

@jeisinger
Copy link

Is there something in this process that will either overwrite or verify the arm64 data? Is my assumption correct that I could craft a payload that looks like a valid serialization but contains an exploit binary in the arm64 bits?

@jfbastien
Copy link
Member

Is there something in this process that will either overwrite or verify the arm64 data? Is my assumption correct that I could craft a payload that looks like a valid serialization but contains an exploit binary in the arm64 bits?

Are we still talking about cross-origin structured cloning, or just structured cloning, or V8's particular implementation of it? Or something else entirely?

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 16, 2017

That's correct, and in that case, unless I'm missing something, the exploit is still executing in the sandbox of the renderer, at least in v8.

@jeisinger
Copy link

jeisinger commented Jan 16, 2017 via email

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 16, 2017

A JS script that fishes out user data. Can the native code-based approach cause more damage than a JS-based attack (in a sandboxed process, of course)?

@jeisinger
Copy link

jeisinger commented Jan 16, 2017 via email

@mtrofin
Copy link
Contributor Author

mtrofin commented Jan 16, 2017

If I understand this correctly, your position is that "an attacker can cause increased harm (when compared to what they may do via script injection) to a user running a website that is not checking message origin in a postMessage handler which freely accepts and runs wasm, if, additionally, the sandbox is also compromised".

Does this capture it?

I suppose it ends up being up to each vendor to evaluate this risk in the context of their implementation and security model. On our side, when @flagxor and I chatted to security, the stance was what he's described earlier. We can tease this further apart on our side on Tuesday (Monday's bank holiday) offline. I'll try setting something up.

@jeisinger
Copy link

No, this does not capture my position. I don't understand where the checking of the origin comes in, nor the compromise of the sandbox.

@annevk
Copy link
Member

annevk commented Feb 2, 2017

Is my assumption correct that I could craft a payload that looks like a valid serialization but contains an exploit binary in the arm64 bits?

How would you be able to do that? Presumably you can only ever message an actual Module instance (which is backed by a bunch of bits specific to the implementation) and only run that. Just like you don't get access to meddle with the details of Blob objects, you wouldn't here either.

@jfbastien
Copy link
Member

I think this will be addressed as part of #1048.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants