1209107 - Don't expose module environment objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

ModuleEnvironmentObject is a type of scope object and is not meant to be exposed to JS in general.  Currently it can be obtained by accessing the |environment| property on a ModuleObject and is used by the self-hosted parts of the module implementation and by the tests.

There should be an intrinsic so that self-hosted JS can access this and testing functions so that tests can check its state, but the object itself should not be exposed like this.

All other ModuleObject properties are either getters for immutable objects or idempotent methods.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1209107-environment

Comment 4

[Shu-yu Guo [:shu]]

Comment on attachment 8666742 [details] [diff] [review]
bug1209107-environment

Review of attachment 8666742 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/ModuleObject.cpp
@@ +762,5 @@
>  js::InitModuleClass(JSContext* cx, HandleObject obj)
>  {
>      static const JSPropertySpec protoAccessors[] = {
> +        //JS_PSG("initialEnvironment", ModuleObject_initialEnvironmentGetter, 0),
> +        //JS_PSG("environment", ModuleObject_environmentGetter, 0),

Should delete instead of commenting out.

::: js/src/builtin/TestingFunctions.cpp
@@ +2902,5 @@
> +        array->initDenseElement(i, StringValue(JSID_TO_STRING(ids[i])));
> +
> +    args.rval().setObject(*array);
> +    return true;
> +}

You already wrote the code for GetModuleEnvironmentNames and GetModuleEnvironmentValue so I have no objections to leaving them in. I don't understand why they were needed though, couldn't you have left the tests the same and only changed uses of module.environment to use the GetModuleEnvironment testing function?

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Shu-yu Guo [:shu] from comment #4)

> couldn't you have left the
> tests the same and only changed uses of module.environment to use the
> GetModuleEnvironment testing function?

The problem is that the fuzzers can then get hold of the scope object and manipulate it in ways we don't support (e.g. add or delete properties).  I've lost the bug where this was happening now, but the point is that these objects are not meant to be exposed to arbitrary script anyway.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Can this land soon? The fuzzbugs for this (duped here) are appearing long enough to be on the verge of being fuzzblockers.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ac68e528d12

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1ac68e528d12