Bug 1774866 - Always allow cloning error stacks. r=nika,sfink,smaug

Differential Revision: https://phabricator.services.mozilla.com/D164577

docshell/base/BrowsingContext.cpp

@@ -2501,8 +2501,6 @@ void BrowsingContext::PostMessageMoz(JSContext* aCx,
   }
 
   JS::CloneDataPolicy clonePolicy;
-  clonePolicy.allowErrorStackFrames();
-
   if (callerInnerWindow && callerInnerWindow->IsSharedMemoryAllowed()) {
     clonePolicy.allowSharedMemoryObjects();
   }

dom/base/PostMessageEvent.cpp

@@ -164,7 +164,6 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHODIMP PostMessageEvent::Run() {
       do_QueryObject(targetWindow);
 
   JS::CloneDataPolicy cloneDataPolicy;
-  cloneDataPolicy.allowErrorStackFrames();
 
   MOZ_DIAGNOSTIC_ASSERT(targetWindow);
   if (mCallerAgentClusterId.isSome() && targetWindow->GetDocGroup() &&
@@ -189,13 +188,7 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHODIMP PostMessageEvent::Run() {
     holder = &mHolder.ref<StructuredCloneHolder>();
   } else {
     MOZ_ASSERT(mHolder.constructed<ipc::StructuredCloneData>());
-    // It's not possible to send shared objects over IPC so we have a different
-    // policy.
-    JS::CloneDataPolicy cloneDataPolicyIPC;
-    cloneDataPolicyIPC.allowErrorStackFrames();
-
-    mHolder.ref<ipc::StructuredCloneData>().Read(cx, &messageData,
-                                                 cloneDataPolicyIPC, rv);
+    mHolder.ref<ipc::StructuredCloneData>().Read(cx, &messageData, rv);
     holder = &mHolder.ref<ipc::StructuredCloneData>();
   }
   if (NS_WARN_IF(rv.Failed())) {

dom/base/StructuredCloneHolder.cpp

@@ -378,10 +378,6 @@ void StructuredCloneHolder::Read(nsIGlobalObject* aGlobal, JSContext* aCx,
                                  const JS::CloneDataPolicy& aCloneDataPolicy,
                                  ErrorResult& aRv) {
   MOZ_ASSERT(aGlobal);
-  // Error stacks require the reading (deserialization) of principals, which is
-  // only possible on the main thread.
-  MOZ_ASSERT_IF(aCloneDataPolicy.areErrorStackFramesAllowed(),
-                NS_IsMainThread());
 
   mozilla::AutoRestore<nsIGlobalObject*> guard(mGlobal);
   auto errorMessageGuard = MakeScopeExit([&] { mErrorMessage.Truncate(); });

dom/base/nsContentUtils.cpp

@@ -10038,10 +10038,6 @@ void nsContentUtils::StructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal,
   if (aGlobal->IsSharedMemoryAllowed()) {
     clonePolicy.allowSharedMemoryObjects();
   }
-  // Stack principals can't be cloned on Worker threads.
-  if (NS_IsMainThread()) {
-    clonePolicy.allowErrorStackFrames();
-  }
 
   StructuredCloneHolder holder(StructuredCloneHolder::CloningSupported,
                                StructuredCloneHolder::TransferringSupported,

dom/base/nsGlobalWindowOuter.cpp

@@ -5955,7 +5955,6 @@ void nsGlobalWindowOuter::PostMessageMozOuter(JSContext* aCx,
       scriptLocation, callerAgentClusterId);
 
   JS::CloneDataPolicy clonePolicy;
-  clonePolicy.allowErrorStackFrames();
 
   if (GetDocGroup() && callerAgentClusterId.isSome() &&
       GetDocGroup()->AgentClusterId().Equals(callerAgentClusterId.value())) {

dom/workers/RuntimeService.cpp

@@ -876,6 +876,7 @@ class WorkerJSContext final : public mozilla::CycleCollectedJSContext {
 
     js::SetPreserveWrapperCallbacks(cx, PreserveWrapper, HasReleasedWrapper);
     JS_InitDestroyPrincipalsCallback(cx, nsJSPrincipals::Destroy);
+    JS_InitReadPrincipalsCallback(cx, nsJSPrincipals::ReadPrincipals);
     JS_SetWrapObjectCallbacks(cx, &WrapObjectCallbacks);
     if (mWorkerPrivate->IsDedicatedWorker()) {
       JS_SetFutexCanWait(cx);

dom/worklet/WorkletThread.cpp

@@ -137,6 +137,7 @@ class WorkletJSContext final : public CycleCollectedJSContext {
 
     js::SetPreserveWrapperCallbacks(cx, PreserveWrapper, HasReleasedWrapper);
     JS_InitDestroyPrincipalsCallback(cx, nsJSPrincipals::Destroy);
+    JS_InitReadPrincipalsCallback(cx, nsJSPrincipals::ReadPrincipals);
     JS_SetWrapObjectCallbacks(cx, &WrapObjectCallbacks);
     JS_SetFutexCanWait(cx);
 

js/public/StructuredClone.h

@@ -216,15 +216,13 @@ enum TransferableOwnership {
 class CloneDataPolicy {
   bool allowIntraClusterClonableSharedObjects_;
   bool allowSharedMemoryObjects_;
-  bool allowErrorStackFrames_;
 
  public:
   // The default is to deny all policy-controlled aspects.
 
   CloneDataPolicy()
       : allowIntraClusterClonableSharedObjects_(false),
-        allowSharedMemoryObjects_(false),
-        allowErrorStackFrames_(false) {}
+        allowSharedMemoryObjects_(false) {}
 
   // SharedArrayBuffers and WASM modules can only be cloned intra-process
   // because the shared memory areas are allocated in process-private memory or
@@ -243,12 +241,6 @@ class CloneDataPolicy {
   bool areSharedMemoryObjectsAllowed() const {
     return allowSharedMemoryObjects_;
   }
-
-  // The Error stack property is saved as SavedFrames, which
-  // have an associated principal. This principal can't be cloned
-  // in certain cases.
-  void allowErrorStackFrames() { allowErrorStackFrames_ = true; }
-  bool areErrorStackFramesAllowed() const { return allowErrorStackFrames_; }
 };
 
 } /* namespace JS */

js/src/builtin/TestingFunctions.cpp

@@ -4875,30 +4875,6 @@ bool js::testingFunc_serialize(JSContext* cx, unsigned argc, Value* vp) {
       }
       clonebuf.emplace(*scope, nullptr, nullptr);
     }
-
-    if (!JS_GetProperty(cx, opts, "ErrorStackFrames", &v)) {
-      return false;
-    }
-
-    if (!v.isUndefined()) {
-      JSString* str = JS::ToString(cx, v);
-      if (!str) {
-        return false;
-      }
-      JSLinearString* poli = str->ensureLinear(cx);
-      if (!poli) {
-        return false;
-      }
-
-      if (StringEqualsLiteral(poli, "allow")) {
-        policy.allowErrorStackFrames();
-      } else if (StringEqualsLiteral(poli, "deny")) {
-        // default
-      } else {
-        JS_ReportErrorASCII(cx, "Invalid policy value for 'ErrorStackFrames'");
-        return false;
-      }
-    }
   }
 
   if (!clonebuf) {
@@ -4996,30 +4972,6 @@ static bool Deserialize(JSContext* cx, unsigned argc, Value* vp) {
 
       scope = *maybeScope;
     }
-
-    if (!JS_GetProperty(cx, opts, "ErrorStackFrames", &v)) {
-      return false;
-    }
-
-    if (!v.isUndefined()) {
-      JSString* str = JS::ToString(cx, v);
-      if (!str) {
-        return false;
-      }
-      JSLinearString* poli = str->ensureLinear(cx);
-      if (!poli) {
-        return false;
-      }
-
-      if (StringEqualsLiteral(poli, "allow")) {
-        policy.allowErrorStackFrames();
-      } else if (StringEqualsLiteral(poli, "deny")) {
-        // default
-      } else {
-        JS_ReportErrorASCII(cx, "Invalid policy value for 'ErrorStackFrames'");
-        return false;
-      }
-    }
   }
 
   // Clone buffer was already consumed?

js/src/jit-test/tests/structured-clone/errors.js

@@ -6,8 +6,7 @@
 load(libdir + "asserts.js");
 
 function roundtrip(error) {
-  let opts = {ErrorStackFrames: "allow"};
-  return deserialize(serialize(error, [], opts), opts);
+  return deserialize(serialize(error, []));
 }
 
 // Basic
@@ -138,28 +137,3 @@ for (let constructor of constructors) {
   assertEq(cloned.errors, undefined);
   assertEq(cloned.hasOwnProperty('errors'), false);
 }
-
-{
-  let error = new Error();
-
-  // When serializing without stack-frames, deserialization is empty.
-  let cloned = deserialize(serialize(error, [], {ErrorStackFrames: "deny"}),
-    {ErrorStackFrames: "allow"});
-  assertEq(cloned.name, "Error");
-  assertEq(cloned.stack, "");
-
-  // Defaults to disallow.
-  cloned = deserialize(serialize(error));
-  assertEq(cloned.name, "Error");
-  assertEq(cloned.stack, "");
-
-  // Unexpected stack frames during deserialization throw.
-  assertErrorMessage(() => {
-    deserialize(serialize(error, [], {ErrorStackFrames: "allow"}),
-      {ErrorStackFrames: "deny"});
-  }, InternalError, "bad serialized structured data (disallowed 'stack' field encountered for Error object)");
-
-  // Sanity check
-  cloned = roundtrip(error);
-  assertEq(cloned.stack.length > 0, true);
-}

js/src/vm/StructuredClone.cpp

@@ -1902,17 +1902,13 @@ bool JSStructuredCloneWriter::traverseError(HandleObject obj) {
   MOZ_ASSERT(unwrapped);
 
   // Non-standard: Serialize |stack|.
-  // The Error stack property is saved as SavedFrames, which
-  // have an associated principal. This principal can't be cloned
-  // in certain cases.
+  // The Error stack property is saved as SavedFrames.
   RootedValue stack(cx, NullValue());
-  if (cloneDataPolicy.areErrorStackFramesAllowed()) {
-    RootedObject stackObj(cx, unwrapped->stack());
-    if (stackObj && stackObj->canUnwrapAs<SavedFrame>()) {
-      stack.setObject(*stackObj);
-      if (!cx->compartment()->wrap(cx, &stack)) {
-        return false;
-      }
+  RootedObject stackObj(cx, unwrapped->stack());
+  if (stackObj && stackObj->canUnwrapAs<SavedFrame>()) {
+    stack.setObject(*stackObj);
+    if (!cx->compartment()->wrap(cx, &stack)) {
+      return false;
     }
   }
   if (!otherEntries.append(stack)) {
@@ -3633,12 +3629,6 @@ bool JSStructuredCloneReader::readErrorFields(Handle<ErrorObject*> errorObj,
                                 "invalid 'stack' field for Error object");
       return false;
     }
-    if (!cloneDataPolicy.areErrorStackFramesAllowed()) {
-      JS_ReportErrorNumberASCII(
-          cx, GetErrorMessage, nullptr, JSMSG_SC_BAD_SERIALIZED_DATA,
-          "disallowed 'stack' field encountered for Error object");
-      return false;
-    }
     errorObj->setStackSlot(stack);
   } else if (!stack.isNull()) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,

testing/web-platform/meta/html/infrastructure/safe-passing-of-structured-data/structured-cloning-error-stack-optional.sub.window.js.ini

@@ -7,18 +7,9 @@
   [web API-created DOMException (structuredClone())]
     expected: FAIL
 
-  [page-created Error (worker)]
-    expected: FAIL
-
   [web API-created DOMException (worker)]
     expected: FAIL
 
-  [JS-engine-created TypeError (worker)]
-    expected: FAIL
-
-  [web API-created TypeError (worker)]
-    expected: FAIL
-
   [page-created DOMException (worker)]
     expected: FAIL