Bug 1042436 - Warn once to the console when XrayWrappers deny access to an object. r=gabor, sr=smaug, a=sledru
💩💩 backed out by 53c7aceaf1a8 💩 💩
authorBobby Holley <bobbyholley@gmail.com>
Wed, 06 Aug 2014 23:32:23 -0400
changeset 208275 1cf7b5810eb5
parent 208274 4cb373d9ce33
child 208276 53c7aceaf1a8
push id3796
push userryanvm@gmail.com
push date2014-08-08 18:52 +0000
treeherdermozilla-beta@1cf7b5810eb5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor, smaug, sledru
bugs1042436
milestone32.0
Bug 1042436 - Warn once to the console when XrayWrappers deny access to an object. r=gabor, sr=smaug, a=sledru
js/xpconnect/src/xpcprivate.h
js/xpconnect/tests/chrome/chrome.ini
js/xpconnect/tests/chrome/test_bug1042436.xul
js/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -3469,16 +3469,17 @@ public:
         LocationHintAddon
     };
 
     CompartmentPrivate(JSCompartment *c)
         : wantXrays(false)
         , universalXPConnectEnabled(false)
         , adoptedNode(false)
         , donatedNode(false)
+        , warnedAboutXrays(false)
         , scriptability(c)
         , scope(nullptr)
     {
         MOZ_COUNT_CTOR(xpc::CompartmentPrivate);
     }
 
     ~CompartmentPrivate();
 
@@ -3489,16 +3490,20 @@ public:
     // enablePrivilege. Once set, this value is never unset (i.e., it doesn't follow
     // the old scoping rules of enablePrivilege). Using it is inherently unsafe.
     bool universalXPConnectEnabled;
 
     // for telemetry. See bug 928476.
     bool adoptedNode;
     bool donatedNode;
 
+    // Whether we've emitted a warning about a property that was filtered out
+    // by XrayWrappers. See XrayWrapper.cpp.
+    bool warnedAboutXrays;
+
     // The scriptability of this compartment.
     Scriptability scriptability;
 
     // Our XPCWrappedNativeScope. This is non-null if and only if this is an
     // XPConnect compartment.
     XPCWrappedNativeScope *scope;
 
     const nsACString& GetLocation() {
--- a/js/xpconnect/tests/chrome/chrome.ini
+++ b/js/xpconnect/tests/chrome/chrome.ini
@@ -50,16 +50,17 @@ support-files =
 [test_bug853283.xul]
 [test_bug853571.xul]
 [test_bug858101.xul]
 [test_bug860494.xul]
 [test_bug866823.xul]
 [test_bug895340.xul]
 [test_bug932906.xul]
 [test_bug996069.xul]
+[test_bug1042436.xul]
 [test_xrayToJS.xul]
 [test_chrometoSource.xul]
 [test_cloneInto.xul]
 [test_cows.xul]
 [test_discardSystemSource.xul]
 [test_documentdomain.xul]
 [test_doublewrappedcompartments.xul]
 [test_evalInSandbox.xul]
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug1042436.xul
@@ -0,0 +1,49 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1042436
+-->
+<window title="Mozilla Bug 1042436"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1042436"
+     target="_blank">Mozilla Bug 1042436</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript">
+  <![CDATA[
+  /** Test for Bug 1042436 **/
+  SimpleTest.waitForExplicitFinish();
+  const Cu = Components.utils;
+
+  var contentSb = Cu.Sandbox('http://www.example.com');
+  var nonXrayableObj = new contentSb.WeakMap();
+  nonXrayableObj.wrappedJSObject.someExpandoProperty = 42;
+  nonXrayableObj.wrappedJSObject.someOtherExpandoProperty = 52;
+
+
+  SimpleTest.expectConsoleMessages(function() {
+      nonXrayableObj.someExpandoProperty;
+      nonXrayableObj.someOtherExpandoProperty;
+
+      var chromeSb = Cu.Sandbox(this);
+      var contentObjWithGetter = contentSb.eval('({ get getterProp() {return 42;}, valueProp: 42 })');
+      is(contentObjWithGetter.wrappedJSObject.getterProp, 42, "Getter prop set up correctly");
+      is(contentObjWithGetter.getterProp, undefined, "Xrays work right");
+      is(contentObjWithGetter.valueProp, 42, "Getter prop set up correctly");
+      chromeSb.contentObjWithGetter = contentObjWithGetter;
+      Cu.evalInSandbox('contentObjWithGetter.getterProp; contentObjWithGetter.valueProp; contentObjWithGetter.getterProp;',
+                       chromeSb, "1.7", "http://phony.example.com/file.js", 99);
+  },
+  [{ errorMessage: /property someExpandoProperty \(reason: object is not safely Xrayable/, sourceName: /test_bug1042436/, isWarning: true },
+   { errorMessage: /property getterProp \(reason: property has accessor/, sourceName: /phony/, lineNumber: 99, isWarning: true } ],
+  SimpleTest.finish.bind(SimpleTest));
+
+  ]]>
+  </script>
+</window>
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "XrayWrapper.h"
 #include "AccessCheck.h"
 #include "WrapperFactory.h"
 
 #include "nsIContent.h"
 #include "nsIControllers.h"
+#include "nsIScriptError.h"
 #include "mozilla/dom/Element.h"
 #include "nsContentUtils.h"
 
 #include "XPCWrapper.h"
 #include "xpcprivate.h"
 
 #include "jsapi.h"
 #include "jsprf.h"
@@ -407,27 +408,79 @@ const JSClass JSXrayTraits::HolderClass 
     JS_PropertyStub, JS_DeletePropertyStub,
     JS_PropertyStub, JS_StrictPropertyStub,
     JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub
 };
 
 inline bool
 SilentFailure(JSContext *cx, HandleId id, const char *reason)
 {
-#ifdef DEBUG
-    nsDependentJSString name;
-    if (!name.init(cx, id))
+    CompartmentPrivate *priv = CompartmentPrivate::Get(CurrentGlobalOrNull(cx));
+    bool alreadyWarnedOnce = priv->warnedAboutXrays;
+    priv->warnedAboutXrays = true;
+
+    // The browser console warning is only emitted for the first violation,
+    // whereas the (debug-only) NS_WARNING is emitted for each violation.
+#ifndef DEBUG
+    if (alreadyWarnedOnce)
+        return true;
+#endif
+
+    nsDependentJSString propertyName;
+    if (!propertyName.init(cx, id))
         return false;
     AutoFilename filename;
     unsigned line = 0;
     DescribeScriptedCaller(cx, &filename, &line);
+
+    // Warn to the terminal for the logs.
     NS_WARNING(nsPrintfCString("Denied access to property |%s| on Xrayed Object: %s (@%s:%u)",
-                               NS_LossyConvertUTF16toASCII(name).get(), reason,
+                               NS_LossyConvertUTF16toASCII(propertyName).get(), reason,
                                filename.get(), line).get());
-#endif
+
+    // If this isn't the first warning on this topic for this global, we've
+    // already bailed out in opt builds. Now that the NS_WARNING is done, bail
+    // out in debug builds as well.
+    if (alreadyWarnedOnce)
+        return true;
+
+    //
+    // Log a message to the console service.
+    //
+
+    // Grab the pieces.
+    nsCOMPtr<nsIConsoleService> consoleService = do_GetService(NS_CONSOLESERVICE_CONTRACTID);
+    NS_ENSURE_TRUE(consoleService, true);
+    nsCOMPtr<nsIScriptError> errorObject = do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
+    NS_ENSURE_TRUE(errorObject, true);
+
+    // Compute the current window id if any.
+    uint64_t windowId = 0;
+    nsGlobalWindow *win = WindowGlobalOrNull(CurrentGlobalOrNull(cx));
+    if (win)
+      windowId = win->WindowID();
+
+    nsPrintfCString errorMessage("XrayWrapper denied access to property %s (reason: %s). "
+                                 "See https://developer.mozilla.org/en-US/docs/Xray_vision "
+                                 "for more information. Note that only the first denied "
+                                 "property access from a given global object will be reported.",
+                                 NS_LossyConvertUTF16toASCII(propertyName).get(),
+                                 reason);
+    nsString filenameStr(NS_ConvertASCIItoUTF16(filename.get()));
+    nsresult rv = errorObject->InitWithWindowID(NS_ConvertASCIItoUTF16(errorMessage),
+                                                filenameStr,
+                                                EmptyString(),
+                                                line, 0,
+                                                nsIScriptError::warningFlag,
+                                                "XPConnect",
+                                                windowId);
+    NS_ENSURE_SUCCESS(rv, true);
+    rv = consoleService->LogMessage(errorObject);
+    NS_ENSURE_SUCCESS(rv, true);
+
     return true;
 }
 
 bool JSXrayTraits::getOwnPropertyFromTargetIfSafe(JSContext *cx,
                                                   HandleObject target,
                                                   HandleObject wrapper,
                                                   HandleId idArg,
                                                   MutableHandle<JSPropertyDescriptor> outDesc)