Bug 29437 - Parsing a JWK can have side-effects if not done very carefully

[mwatson2]

Bug 29437:

The algorithm in https://www.w3.org/TR/WebCryptoAPI/#concept-parse-a-jwk will, if executed in the page global, generally run getters from Object.prototype for any fields missing in the JSON during the IDL dictionary type conversion. Is this the intended behavior? Or is the intent that this parsing happens in some global other than the page global, where things like this won't be observable? Clearly defining this, either way, would be a good idea.

[mwatson2]

@bzbarsky I think it would be odd for things from Object.prototype to affect the JWK conversion to WebIDL dictionary.

This (and #85) seem to be examples of a more general problem that none of WebCrypto, WebIDL nor even ECMAScript (for example here) specify which global the operations take place in, or mention the need to specify this.

Would it suffice to provide some words around the use of JSON.parse such that the created object has either no prototype (i.e. start with Object.create(null)) or prototype equal to the original value of Object.prototype ?

[mwatson2]

@bzbarsky @sleevi I caught up with the discussion on the original bug, but I don't believe it concluded.

The approach of creating objects with null prototypes seems appropriate, since I think we would want to get an exception if a JWK member that should be a string is in fact an Object or and Array. But I understand that a blanket monkey-patch to JSON.parse to create all objects this way will break things for Arrays, right ?

Would a reviver function that nulls out the prototypes of things that are Objects but not Arrays work ? I guess you are still left with properties from the global Array.prototype...

Any ideas ?

[bzbarsky]

none of WebCrypto, WebIDL nor even ECMAScript (for example here) specify which global the operations take place in

WebIDL totally specifies it in the cases when it's actually calling out to actual ES functions (which is almost never).

ES also specifies it; for example their %whatever% notation is defined to mean the thing with that name from the global of the current Realm.

or mention the need to specify this.

Where would one mention this? It's pretty obvious that if you're going to call JS things you have to say which global they come from, because most JS things have different behavior based on their global.

Would it suffice to provide some words around the use of JSON.parse such that the created object has either no prototype (i.e. start with Object.create(null)) or prototype equal to the original value of Object.prototype ?

I'm not sure what that would mean. If you're invoking http://www.ecma-international.org/ecma-262/6.0/#sec-json.parse then you get whatever behavior it produces. If you're NOT invoking it, you need to define what actual algorithm you're invoking and what it does.

Just nulling out the prototype of the object itself is not enough if you want to do all this in the page's global, because JsonWebKey has sequence members, and if you work in the global of the page it can override Array.prototype[Symbol.iterator] as well as various other objects involved in array iteration.

For what it's worth, Gecko implements this thing by doing the JSON.parse in a clean global that the page cannot touch.

But I understand that a blanket monkey-patch to JSON.parse to create all objects this way will break things for Arrays, right ?

Yes.

Would a reviver function that nulls out the prototypes of things that are Objects but not Arrays work ?

No, because then the page can hook the array iterator bits.

Any ideas ?

Define all this stuff in terms of doing the JSON.parse and JSON.serialize in a clean global. I think that's the only sane thing to do.

[mwatson2]

@bzbarsky Thanks! Is there a particular form of words to specify a "clean global" or is that the right terminology already.

[bzbarsky]

I think if you want to be very pedantic, then you do http://www.ecma-international.org/ecma-262/6.0/#sec-createrealm followed by http://www.ecma-international.org/ecma-262/6.0/#sec-initializehostdefinedrealm and then use the global object of that Realm.

[mwatson2]

@bzbarsky If I don't want to be very pedantic, is "JSON.parse in the context of a new global object" sufficient ?

When I said:

none of WebCrypto, WebIDL nor even ECMAScript (for example here) specify which global the operations take place in or mention the need to specify this.

I did not articulate my point very well. Sorry.

A better way to put it would be to note that the fundamental mutability of basic language features in Javascript is something that, as a Javascript programmer, one can choose not to utilize (and in fact it has long been discouraged). But as a spec writer it keeps coming up. In my experience there is approximately one person (your good self) who notices these real problems ;-) It would be helpful if functions in ECMAScript or WebIDL that might reasonably be called from other specifications were, well, functional: that is, specified all their dependencies in their signatures. This would be an immediate and obvious sign to the caller that they needed to take care of this.

[bzbarsky]

If I don't want to be very pedantic, is "JSON.parse in the context of a new global object" sufficient ?

Probably, yes. It would at least make an implementor stop and notice the problem.

It would be helpful if functions in ECMAScript or WebIDL that might reasonably be called from other specifications were, well, functional: that is, specified all their dependencies in their signatures.

Hmm. Yeah, the problem is that ES basically always has an implicit argument: the global of the function's Realm. :(

[mwatson2]

PR #115