Bug 807548. Enumerating an Xray should not see the 'constructor' property on DOM prototypes unless we're told to include non-enumerable properties. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 05 Nov 2012 11:58:03 -0500
changeset 112324 af47a345a5be1d72bdb56c524ed98d446bd30864
parent 112323 b74aba46826fbc60b742065aa1983d10e7be809f
child 112325 9bb44a0caae4c4d1db19cd7fabe541915e71d8a0
push id23812
push useremorley@mozilla.com
push dateTue, 06 Nov 2012 14:01:34 +0000
treeherdermozilla-central@f4aeed115e54 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs807548
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 807548. Enumerating an Xray should not see the 'constructor' property on DOM prototypes unless we're told to include non-enumerable properties. r=peterv
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/tests/mochitest/chrome/test_sandbox_bindings.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -806,36 +806,38 @@ XrayResolveNativeProperty(JSContext* cx,
 
   return XrayResolveNativeProperty(cx, wrapper, nativePropertyHooks, type, obj,
                                    id, desc);
 }
 
 bool
 XrayEnumerateAttributes(Prefable<JSPropertySpec>* attributes,
                         jsid* attributeIds, JSPropertySpec* attributeSpecs,
-                        JS::AutoIdVector& props)
+                        unsigned flags, JS::AutoIdVector& props)
 {
   for (; attributes->specs; ++attributes) {
     if (attributes->enabled) {
       // Set i to be the index into our full list of ids/specs that we're
       // looking at now.
       size_t i = attributes->specs - attributeSpecs;
       for ( ; attributeIds[i] != JSID_VOID; ++i) {
-        if ((attributeSpecs[i].flags & JSPROP_ENUMERATE) &&
+        if (((flags & JSITER_HIDDEN) ||
+             (attributeSpecs[i].flags & JSPROP_ENUMERATE)) &&
             !props.append(attributeIds[i])) {
           return false;
         }
       }
     }
   }
   return true;
 }
 
 bool
-XrayEnumerateProperties(JS::AutoIdVector& props, DOMObjectType type,
+XrayEnumerateProperties(unsigned flags, JS::AutoIdVector& props,
+                        DOMObjectType type,
                         const NativeProperties* nativeProperties)
 {
   Prefable<JSFunctionSpec>* methods;
   jsid* methodIds;
   JSFunctionSpec* methodsSpecs;
   if (type == eInterface) {
     methods = nativeProperties->staticMethods;
     methodIds = nativeProperties->staticMethodIds;
@@ -848,45 +850,47 @@ XrayEnumerateProperties(JS::AutoIdVector
   if (methods) {
     Prefable<JSFunctionSpec>* method;
     for (method = methods; method->specs; ++method) {
       if (method->enabled) {
         // Set i to be the index into our full list of ids/specs that we're
         // looking at now.
         size_t i = method->specs - methodsSpecs;
         for ( ; methodIds[i] != JSID_VOID; ++i) {
-          if ((methodsSpecs[i].flags & JSPROP_ENUMERATE) &&
+          if (((flags & JSITER_HIDDEN) ||
+               (methodsSpecs[i].flags & JSPROP_ENUMERATE)) &&
               !props.append(methodIds[i])) {
             return false;
           }
         }
       }
     }
   }
 
   if (type == eInterface) {
     if (nativeProperties->staticAttributes &&
         !XrayEnumerateAttributes(nativeProperties->staticAttributes,
                                  nativeProperties->staticAttributeIds,
                                  nativeProperties->staticAttributeSpecs,
-                                 props)) {
+                                 flags, props)) {
       return false;
     }
   } else {
     if (nativeProperties->attributes &&
         !XrayEnumerateAttributes(nativeProperties->attributes,
                                  nativeProperties->attributeIds,
-                                 nativeProperties->attributeSpecs, props)) {
+                                 nativeProperties->attributeSpecs,
+                                 flags, props)) {
       return false;
     }
     if (nativeProperties->unforgeableAttributes &&
         !XrayEnumerateAttributes(nativeProperties->unforgeableAttributes,
                                  nativeProperties->unforgeableAttributeIds,
                                  nativeProperties->unforgeableAttributeSpecs,
-                                 props)) {
+                                 flags, props)) {
       return false;
     }
   }
 
   if (nativeProperties->constants) {
     Prefable<ConstantSpec>* constant;
     for (constant = nativeProperties->constants; constant->specs; ++constant) {
       if (constant->enabled) {
@@ -904,84 +908,89 @@ XrayEnumerateProperties(JS::AutoIdVector
 
   return true;
 }
 
 bool
 XrayEnumerateNativeProperties(JSContext* cx, JSObject* wrapper,
                               const NativePropertyHooks* nativePropertyHooks,
                               DOMObjectType type, JSObject* obj,
-                              JS::AutoIdVector& props)
+                              unsigned flags, JS::AutoIdVector& props)
 {
   if (type == eInterface &&
       nativePropertyHooks->mPrototypeID != prototypes::id::_ID_Count &&
       !AddStringToIDVector(cx, props, "prototype")) {
     return false;
   }
 
   if (type == eInterfacePrototype &&
       nativePropertyHooks->mConstructorID != constructors::id::_ID_Count &&
+      (flags & JSITER_HIDDEN) &&
       !AddStringToIDVector(cx, props, "constructor")) {
     return false;
   }
 
   const NativePropertiesHolder& nativeProperties =
     nativePropertyHooks->mNativeProperties;
 
   if (nativeProperties.regular &&
-      !XrayEnumerateProperties(props, type, nativeProperties.regular)) {
+      !XrayEnumerateProperties(flags, props, type, nativeProperties.regular)) {
     return false;
   }
 
   if (nativeProperties.chromeOnly &&
       xpc::AccessCheck::isChrome(js::GetObjectCompartment(wrapper)) &&
-      !XrayEnumerateProperties(props, type, nativeProperties.chromeOnly)) {
+      !XrayEnumerateProperties(flags, props, type, nativeProperties.chromeOnly)) {
     return false;
   }
 
   return true;
 }
 
 bool
 XrayEnumerateProperties(JSContext* cx, JSObject* wrapper, JSObject* obj,
-                        bool ownOnly, JS::AutoIdVector& props)
+                        unsigned flags, JS::AutoIdVector& props)
 {
   DOMObjectType type;
   const NativePropertyHooks* nativePropertyHooks =
     GetNativePropertyHooks(cx, obj, type);
 
   if (type == eInstance) {
     if (nativePropertyHooks->mEnumerateOwnProperties &&
         !nativePropertyHooks->mEnumerateOwnProperties(cx, wrapper, obj,
                                                       props)) {
       return false;
     }
 
-    if (ownOnly) {
+    if (flags & JSITER_OWNONLY) {
       return true;
     }
 
     // Force the type to be eInterfacePrototype, since we need to walk the
     // prototype chain.
     type = eInterfacePrototype;
   }
 
   if (type == eInterfacePrototype) {
     do {
       if (!XrayEnumerateNativeProperties(cx, wrapper, nativePropertyHooks, type,
-                                         obj, props)) {
+                                         obj, flags, props)) {
         return false;
       }
+
+      if (flags & JSITER_OWNONLY) {
+        return true;
+      }
     } while ((nativePropertyHooks = nativePropertyHooks->mProtoHooks));
 
     return true;
   }
 
   return XrayEnumerateNativeProperties(cx, wrapper, nativePropertyHooks, type,
-                                       obj, props);
+                                       obj, flags, props);
 }
 
 NativePropertyHooks sWorkerNativePropertyHooks = {
   nullptr,
   nullptr,
   {
     nullptr,
     nullptr
@@ -1158,17 +1167,18 @@ NativeToString(JSContext* cx, JSObject* 
     } else {
       if (IsDOMProxy(obj)) {
         str = js::GetProxyHandler(obj)->obj_toString(cx, obj);
       } else if (IsDOMClass(JS_GetClass(obj)) ||
                  IsDOMIfaceAndProtoClass(JS_GetClass(obj))) {
         str = ConcatJSString(cx, "[object ",
                              JS_NewStringCopyZ(cx, JS_GetClass(obj)->name),
                                                "]");
-      } else if (JS_IsNativeFunction(obj, Constructor)) {
+      } else {
+        MOZ_ASSERT(JS_IsNativeFunction(obj, Constructor));
         str = JS_DecompileFunction(cx, JS_GetObjectFunction(obj), 0);
       }
       str = ConcatJSString(cx, pre, str, post);
     }
   }
 
   if (!str) {
     return false;
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1309,20 +1309,21 @@ XrayResolveNativeProperty(JSContext* cx,
 
 /**
  * This enumerates indexed or named properties of obj and operations, attributes
  * and constants of the interfaces for obj.
  *
  * wrapper is the Xray JS object.
  * obj is the target object of the Xray, a binding's instance object or a
  *     interface or interface prototype object.
+ * flags are JSITER_* flags.
  */
 bool
 XrayEnumerateProperties(JSContext* cx, JSObject* wrapper, JSObject* obj,
-                        bool ownOnly, JS::AutoIdVector& props);
+                        unsigned flags, JS::AutoIdVector& props);
 
 extern NativePropertyHooks sWorkerNativePropertyHooks;
 
 // We use one constructor JSNative to represent all DOM interface objects (so
 // we can easily detect when we need to wrap them in an Xray wrapper). We store
 // the real JSNative in the mNative member of a JSNativeHolder in the
 // CONSTRUCTOR_NATIVE_HOLDER_RESERVED_SLOT slot of the JSFunction object for a
 // specific interface object. We also store the NativeProperties in the
--- a/dom/tests/mochitest/chrome/test_sandbox_bindings.xul
+++ b/dom/tests/mochitest/chrome/test_sandbox_bindings.xul
@@ -87,16 +87,34 @@ https://bugzilla.mozilla.org/show_bug.cg
         ok(false, "'XMLHttpRequest()' shouldn't throw in a sandbox");
       }
       try {
         var xhr = Components.utils.evalInSandbox("XMLHttpRequest.prototype.toString = function () { return 'Failed'; }; XMLHttpRequest();", sandbox);
         is(xhr.toString(), "[object XrayWrapper " + XMLHttpRequest() + "]", "XMLHttpRequest.prototype.toString in the sandbox should not override the native toString behaviour");
       } catch (e) {
         ok(false, "'new XMLHttpRequest()' shouldn't throw in a sandbox");
       }
+
+      try {
+        // have to run this test before document.defaultView.XMLHttpRequest
+        // gets munged in the sandbox.
+        var proto = Components.utils.evalInSandbox("XMLHttpRequest.prototype", sandbox);
+        props = [];
+        for (var i in proto) {
+          props.push(i);
+        }
+        isnot(props.indexOf("dispatchEvent"), -1,
+           "'dispatchEvent' property should be enumerable on XMLHttpRequest.prototype");
+        props = Object.getOwnPropertyNames(proto);
+        is(props.indexOf("dispatchEvent"), -1,
+           "'dispatchEvent' is not an own property on XMLHttpRequest.prototype; it's on EventTarget.prototype")
+      } catch (e) {
+        ok(false, "XMLHttpRequest.prototype manipulation via an Xray shouldn't throw" + e);
+      }
+
       try {
         Components.utils.evalInSandbox("document.defaultView.XMLHttpRequest = function() {};", sandbox);
         var win = Components.utils.evalInSandbox("document.defaultView", sandbox);
         var xhr = win.XMLHttpRequest();
         is("" + xhr, "" + XMLHttpRequest(), "'XMLHttpRequest()' in a sandbox should create an XMLHttpRequest object");
       } catch (e) {
         ok(false, "'XMLHttpRequest()' shouldn't throw in a sandbox");
       }
@@ -127,16 +145,47 @@ https://bugzilla.mozilla.org/show_bug.cg
         ctx.putImageData(data, 0, 0);
         var data2 = ctx.getImageData(0, 0, 1, 1);
         is(data2.data.length, data.data.length, "Lengths must match");
         for (i = 0; i < data.data.length; ++i)
           is(data.data[i], data2.data[i], "Data at " + i + " should match");
       } catch (e) {
         ok(false, "Imagedata manipulation via an Xray shouldn't throw " + e);
       }
+
+      try {
+        var list = Components.utils.evalInSandbox("document.getElementsByTagName('*')", sandbox);
+        props = [];
+        for (var i in list) {
+          props.push(i);
+        }
+        is(props.indexOf("constructor"), -1,
+           "'constructor' property should not be enumerable on list object");
+        props = Object.getOwnPropertyNames(list);
+        is(props.indexOf("constructor"), -1,
+           "'constructor' property should not be an own property name on list object");
+      } catch (e) {
+        ok(false, "NodeList.prototype manipulation via an Xray shouldn't throw" + e);
+      }
+
+      try {
+        var proto = Components.utils.evalInSandbox("NodeList.prototype", sandbox);
+        props = [];
+        for (var i in proto) {
+          props.push(i);
+        }
+        is(props.indexOf("constructor"), -1,
+           "'constructor' property should not be enumerable on proto directly");
+        props = Object.getOwnPropertyNames(proto);
+        isnot(props.indexOf("constructor"), -1,
+              "'constructor' property should be an own property name on proto");
+      } catch (e) {
+        ok(false, "NodeList.prototype manipulation via an Xray shouldn't throw" + e);
+      }
+
       SimpleTest.finish();
     }
 
     SimpleTest.waitForExplicitFinish();
     addLoadEvent(doTest);
   ]]>
   </script>
 </window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -1208,18 +1208,17 @@ DOMXrayTraits::resolveOwnProperty(JSCont
     return true;
 }
 
 bool
 DOMXrayTraits::enumerateNames(JSContext *cx, JSObject *wrapper, unsigned flags,
                               JS::AutoIdVector &props)
 {
     return XrayEnumerateProperties(cx, wrapper, getTargetObject(wrapper),
-                                   flags & (JSITER_OWNONLY | JSITER_HIDDEN),
-                                   props);
+                                   flags, props);
 }
 
 bool
 DOMXrayTraits::call(JSContext *cx, JSObject *wrapper, unsigned argc, Value *vp)
 {
     JSObject *obj = getTargetObject(wrapper);
     AutoValueRooter rval(cx);
     bool ok;