Bug 1107953 part 5. Add tests for promise rejections with content-side DOMExceptions, and fix the promise code so those tests actually pass. r=bholley

dom/base/DOMException.cpp

@@ -722,5 +722,32 @@ DOMException::Create(nsresult aRv)
   return inst.forget();
 }
 
+bool
+DOMException::Sanitize(JSContext* aCx,
+                       JS::MutableHandle<JS::Value> aSanitizedValue)
+{
+  nsRefPtr<DOMException> retval = this;
+  if (mLocation && !mLocation->CallerSubsumes(aCx)) {
+    nsString message;
+    GetMessageMoz(message);
+    nsString name;
+    GetName(name);
+    retval = new dom::DOMException(nsresult(Result()),
+                                   NS_ConvertUTF16toUTF8(message),
+                                   NS_ConvertUTF16toUTF8(name),
+                                   Code());
+    // Now it's possible that the stack on retval still starts with
+    // stuff aCx is not supposed to touch; it depends on what's on the
+    // stack right this second.  Walk past all of that.
+    while (retval->mLocation && !retval->mLocation->CallerSubsumes(aCx)) {
+      nsCOMPtr<nsIStackFrame> caller;
+      retval->mLocation->GetCaller(getter_AddRefs(caller));
+      retval->mLocation.swap(caller);
+    }
+  }
+
+  return ToJSValue(aCx, retval, aSanitizedValue);
+}
+
 } // namespace dom
 } // namespace mozilla

dom/base/DOMException.h

@@ -156,6 +156,15 @@ public:
   static already_AddRefed<DOMException>
   Create(nsresult aRv);
 
+  // Sanitize() is a workaround for the fact that DOMExceptions can leak stack
+  // information for the first stackframe to callers that should not have access
+  // to it.  To prevent this, we check whether aCx subsumes our first stackframe
+  // and if not hand out a JS::Value for a clone of ourselves.  Otherwise we
+  // hand out a JS::Value for ourselves.
+  //
+  // If the return value is false, an exception was thrown on aCx.
+  bool Sanitize(JSContext* aCx, JS::MutableHandle<JS::Value> aSanitizedValue);
+
 protected:
 
   virtual ~DOMException() {}

dom/bindings/BindingUtils.cpp

@@ -234,20 +234,11 @@ ErrorResult::ReportJSExceptionFromJSImplementation(JSContext* aCx)
   nsresult rv =
     UNWRAP_OBJECT(DOMException, &mJSException.toObject(), domException);
   if (NS_SUCCEEDED(rv)) {
-    // We actually have to create a new DOMException object, because the one we
+    // We may have to create a new DOMException object, because the one we
     // have has a stack that includes the chrome code that threw it, and in
     // particular has the wrong file/line/column information.
-    nsString message;
-    domException->GetMessageMoz(message);
-    nsString name;
-    domException->GetName(name);
-    nsRefPtr<dom::DOMException> newException =
-      new dom::DOMException(nsresult(domException->Result()),
-                            NS_ConvertUTF16toUTF8(message),
-                            NS_ConvertUTF16toUTF8(name),
-                            domException->Code());
     JS::Rooted<JS::Value> reflector(aCx);
-    if (!GetOrCreateDOMReflector(aCx, newException, &reflector)) {
+    if (!domException->Sanitize(aCx, &reflector)) {
       // Well, that threw _an_ exception.  Let's forget ours.  We can just
       // unroot and not change the value, since mJSException is completely
       // ignored if mResult is not NS_ERROR_DOM_JS_EXCEPTION and we plan to
@@ -301,6 +292,17 @@ ErrorResult::StealJSException(JSContext* cx,
   value.set(mJSException);
   js::RemoveRawValueRoot(cx, &mJSException);
   mResult = NS_OK;
+
+  if (value.isObject()) {
+    // If it's a DOMException we may need to sanitize it.
+    dom::DOMException* domException;
+    nsresult rv =
+      UNWRAP_OBJECT(DOMException, &value.toObject(), domException);
+    if (NS_SUCCEEDED(rv) && !domException->Sanitize(cx, value)) {
+      JS_GetPendingException(cx, value);
+      JS_ClearPendingException(cx);
+    }
+  }
 }
 
 void

dom/bindings/ErrorResult.h

@@ -97,7 +97,8 @@ public:
 
   // StealJSException steals the JS Exception from the object. This method must
   // be called only if IsJSException() returns true. This method also resets the
-  // ErrorCode() to NS_OK.
+  // ErrorCode() to NS_OK.  The value will be ensured to be sanitized wrt to the
+  // current compartment of cx if it happens to be a DOMException.
   void StealJSException(JSContext* cx, JS::MutableHandle<JS::Value> value);
 
   void MOZ_ALWAYS_INLINE MightThrowJSException()

dom/bindings/Exceptions.cpp

@@ -286,6 +286,7 @@ public:
   NS_IMETHOD GetName(nsAString& aFunction) MOZ_OVERRIDE;
   NS_IMETHOD GetCaller(nsIStackFrame** aCaller) MOZ_OVERRIDE;
   NS_IMETHOD GetFormattedStack(nsAString& aStack) MOZ_OVERRIDE;
+  virtual bool CallerSubsumes(JSContext* aCx) MOZ_OVERRIDE;
 
 protected:
   virtual bool IsJSFrame() const MOZ_OVERRIDE {
@@ -613,6 +614,37 @@ NS_IMETHODIMP StackFrame::ToString(nsACString& _retval)
   return NS_OK;
 }
 
+/* virtual */ bool
+StackFrame::CallerSubsumes(JSContext* aCx)
+{
+  return true;
+}
+
+/* virtual */ bool
+JSStackFrame::CallerSubsumes(JSContext* aCx)
+{
+  if (!NS_IsMainThread()) {
+    return true;
+  }
+
+  if (!mStack) {
+    // No problem here, there's no data to leak.
+    return true;
+  }
+
+  nsIPrincipal* callerPrincipal = nsContentUtils::SubjectPrincipal();
+
+  JS::Rooted<JSObject*> unwrappedStack(aCx, js::CheckedUnwrap(mStack));
+  if (!unwrappedStack) {
+    // We can't leak data here either.
+    return true;
+  }
+
+  nsIPrincipal* stackPrincipal =
+    nsJSPrincipals::get(js::GetSavedFramePrincipals(unwrappedStack));
+  return callerPrincipal->SubsumesConsideringDomain(stackPrincipal);
+}
+
 /* static */ already_AddRefed<nsIStackFrame>
 JSStackFrame::CreateStack(JSContext* aCx, int32_t aMaxDepth)
 {

dom/bindings/test/TestInterfaceJS.js

@@ -83,6 +83,13 @@ TestInterfaceJS.prototype = {
       return new this._win.Promise(func);
   },
 
+  testPromiseWithDOMExceptionThrowingPromiseInit: function() {
+    return new this._win.Promise(() => {
+      throw new this._win.DOMException("We are a second DOMException",
+                                       "NotFoundError");
+    })
+  },
+
   testPromiseWithThrowingChromeThenFunction: function() {
     return this._win.Promise.resolve(5).then(function() {
       noSuchMethodExistsYo2();
@@ -93,6 +100,13 @@ TestInterfaceJS.prototype = {
     return this._win.Promise.resolve(10).then(func);
   },
 
+  testPromiseWithDOMExceptionThrowingThenFunction: function() {
+    return this._win.Promise.resolve(5).then(() => {
+      throw new this._win.DOMException("We are a third DOMException",
+                                       "NetworkError");
+    });
+  },
+
   testPromiseWithThrowingChromeThenable: function() {
     // We need to produce a thing that has a "then" property in the page
     // compartment, since we plan to call the page-provided resolve function.
@@ -112,6 +126,20 @@ TestInterfaceJS.prototype = {
     // property won't return the function.
     return this._win.Promise.resolve(Cu.waiveXrays(thenable));
   },
+
+  testPromiseWithDOMExceptionThrowingThenable: function() {
+    // We need to produce a thing that has a "then" property in the page
+    // compartment, since we plan to call the page-provided resolve function.
+    var thenable = new this._win.Object();
+    Cu.waiveXrays(thenable).then = () => {
+      throw new this._win.DOMException("We are a fourth DOMException",
+                                       "TypeMismatchError");
+    }
+    return new this._win.Promise(function(resolve) {
+      resolve(thenable)
+    });
+  },
+
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestInterfaceJS])

dom/bindings/test/test_promise_rejections_from_jsimplemented.html

@@ -14,17 +14,21 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
 
   SimpleTest.waitForExplicitFinish();
 
-  function checkExn(lineNumber, message, filename, testNumber, exn) {
+  function checkExn(lineNumber, name, message, code, filename, testNumber, stack, exn) {
     ise(exn.lineNumber, lineNumber,
         "Should have the right line number in test " + testNumber);
+    ise(exn.name, name,
+        "Should have the right exception name in test " + testNumber);
     ise("filename" in exn ? exn.filename : exn.fileName, filename,
         "Should have the right file name in test " + testNumber);
     ise(exn.message, message,
         "Should have the right message in test " + testNumber);
+    ise(exn.code, code, "Should have the right .code in test " + testNumber);
     if (message === "") {
       ise(exn.name, "NS_ERROR_UNEXPECTED",
           "Should have one of our synthetic exceptions in test " + testNumber);
     }
+    ise(exn.stack, stack, "Should have the right stack in test " + testNumber);
   }
 
   function ensurePromiseFail(testNumber, value) {
@@ -39,34 +43,55 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
     Promise.all([
       t.testPromiseWithThrowingChromePromiseInit().then(
           ensurePromiseFail.bind(null, 1),
-          checkExn.bind(null, 40, "", ourFile, 1)),
+          checkExn.bind(null, 44, "NS_ERROR_UNEXPECTED", "", undefined,
+                        ourFile, 1,
+                        "doTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:44:6\n")),
       t.testPromiseWithThrowingContentPromiseInit(function() {
           thereIsNoSuchContentFunction1();
         }).then(
           ensurePromiseFail.bind(null, 2),
-          checkExn.bind(null, 44,
+          checkExn.bind(null, 50, "ReferenceError",
                         "thereIsNoSuchContentFunction1 is not defined",
-                        ourFile, 2)),
+                        undefined, ourFile, 2,
+                        "doTest/<@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:50:11\ndoTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:49:7\n")),
       t.testPromiseWithThrowingChromeThenFunction().then(
           ensurePromiseFail.bind(null, 3),
-          checkExn.bind(null, 0, "", "", 3)),
+          checkExn.bind(null, 0, "NS_ERROR_UNEXPECTED", "", undefined, "", 3, "")),
       t.testPromiseWithThrowingContentThenFunction(function() {
           thereIsNoSuchContentFunction2();
         }).then(
           ensurePromiseFail.bind(null, 4),
-          checkExn.bind(null, 54,
+          checkExn.bind(null, 61, "ReferenceError",
                         "thereIsNoSuchContentFunction2 is not defined",
-                        ourFile, 4)),
+                        undefined, ourFile, 4,
+                        "doTest/<@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:61:11\n")),
       t.testPromiseWithThrowingChromeThenable().then(
           ensurePromiseFail.bind(null, 5),
-          checkExn.bind(null, 0, "", "", 5)),
+          checkExn.bind(null, 0, "NS_ERROR_UNEXPECTED", "", undefined, "", 5, "")),
       t.testPromiseWithThrowingContentThenable({
             then: function() { thereIsNoSuchContentFunction3(); }
         }).then(
           ensurePromiseFail.bind(null, 6),
-          checkExn.bind(null, 64,
+          checkExn.bind(null, 72, "ReferenceError",
                         "thereIsNoSuchContentFunction3 is not defined",
-                        ourFile, 6)),
+                        undefined, ourFile, 6,
+                        "doTest/<.then@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:72:32\n")),
+      t.testPromiseWithDOMExceptionThrowingPromiseInit().then(
+          ensurePromiseFail.bind(null, 7),
+          checkExn.bind(null, 79, "NotFoundError",
+                        "We are a second DOMException",
+                        DOMException.NOT_FOUND_ERR, ourFile, 7,
+                        "doTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:79:6\n")),
+      t.testPromiseWithDOMExceptionThrowingThenFunction().then(
+          ensurePromiseFail.bind(null, 8),
+          checkExn.bind(null, 0, "NetworkError",
+                        "We are a third DOMException",
+                        DOMException.NETWORK_ERR, "", 8, "")),
+      t.testPromiseWithDOMExceptionThrowingThenable().then(
+          ensurePromiseFail.bind(null, 9),
+          checkExn.bind(null, 0, "TypeMismatchError",
+                        "We are a fourth DOMException",
+                         DOMException.TYPE_MISMATCH_ERR, "", 9, "")),
     ]).then(SimpleTest.finish,
             function() {
               ok(false, "One of our catch statements totally failed");

dom/promise/Promise.cpp

@@ -212,6 +212,11 @@ protected:
     if (rv.Failed()) {
       JS::Rooted<JS::Value> exn(cx);
       if (rv.IsJSException()) {
+        // Enter the compartment of mPromise before stealing the JS exception,
+        // since the StealJSException call will use the current compartment for
+        // a security check that determines how much of the stack we're allowed
+        // to see and we'll be exposing that stack to consumers of mPromise.
+        JSAutoCompartment ac(cx, mPromise->GlobalJSObject());
         rv.StealJSException(cx, &exn);
       } else {
         // Convert the ErrorResult to a JS exception object that we can reject
@@ -579,7 +584,14 @@ Promise::CallInitFunction(const GlobalObject& aGlobal,
 
   if (aRv.IsJSException()) {
     JS::Rooted<JS::Value> value(cx);
-    aRv.StealJSException(cx, &value);
+    { // scope for ac
+      // Enter the compartment of our global before stealing the JS exception,
+      // since the StealJSException call will use the current compartment for
+      // a security check that determines how much of the stack we're allowed
+      // to see, and we'll be exposing that stack to consumers of this promise.
+      JSAutoCompartment ac(cx, GlobalJSObject());
+      aRv.StealJSException(cx, &value);
+    }
 
     // we want the same behavior as this JS implementation:
     // function Promise(arg) { try { arg(a, b); } catch (e) { this.reject(e); }}

dom/promise/PromiseCallback.cpp

@@ -214,7 +214,15 @@ WrapperPromiseCallback::Call(JSContext* aCx,
   if (rv.Failed()) {
     JS::Rooted<JS::Value> value(aCx);
     if (rv.IsJSException()) {
-      rv.StealJSException(aCx, &value);
+      { // scope for ac
+        // Enter the compartment of mNextPromise before stealing the JS
+        // exception, since the StealJSException call will use the current
+        // compartment for a security check that determines how much of the
+        // stack we're allowed to see and we'll be exposing that stack to
+        // consumers of mPromise.
+        JSAutoCompartment ac(aCx, mNextPromise->GlobalJSObject());
+        rv.StealJSException(aCx, &value);
+      }
 
       if (!JS_WrapValue(aCx, &value)) {
         NS_WARNING("Failed to wrap value into the right compartment.");

dom/webidl/TestInterfaceJS.webidl

@@ -59,8 +59,11 @@ interface TestInterfaceJS {
   // Tests for promise-rejection behavior
   Promise<void> testPromiseWithThrowingChromePromiseInit();
   Promise<void> testPromiseWithThrowingContentPromiseInit(PromiseInit func);
+  Promise<void> testPromiseWithDOMExceptionThrowingPromiseInit();
   Promise<void> testPromiseWithThrowingChromeThenFunction();
   Promise<void> testPromiseWithThrowingContentThenFunction(AnyCallback func);
+  Promise<void> testPromiseWithDOMExceptionThrowingThenFunction();
   Promise<void> testPromiseWithThrowingChromeThenable();
   Promise<void> testPromiseWithThrowingContentThenable(object thenable);
+  Promise<void> testPromiseWithDOMExceptionThrowingThenable();
 };

xpcom/base/nsIException.idl

@@ -10,7 +10,9 @@
 
 #include "nsISupports.idl"
 
-[scriptable, uuid(f80ac10b-68dd-4482-a8c2-3cbe13fa89af)]
+[ptr] native JSContext(JSContext);
+
+[scriptable, uuid(4f2927e5-ae85-4c6b-93d2-2f639af6a236)]
 interface nsIStackFrame : nsISupports
 {
     // see nsIProgrammingLanguage for list of language consts
@@ -30,6 +32,10 @@ interface nsIStackFrame : nsISupports
     readonly attribute AString                 formattedStack;
 
     AUTF8String toString();
+
+    // Return whether this stack frame can be accessed by the caller.  This is
+    // safe to call on non-main threads, but will always report "yes" there.
+    [noscript, notxpcom, nostdcall] boolean callerSubsumes(in JSContext aCx);
 };
 
 [scriptable, uuid(1caf1461-be1d-4b79-a552-5292b6bf3c35)]