Bug 1624266 - Add code to page/worker/worklet code so that the global "SharedArrayBuffer" property can be trivially omitted from their global objects by changing how a single C++ variable for each case is initialized. r=baku
authorJeff Walden <jwalden@mit.edu>
Fri, 17 Apr 2020 08:21:02 +0000
changeset 524565 e1d33e7f9caee671ac40b60cfd0621181fb8f04c
parent 524564 7258f2cac07e89a245996fb3054f28cd3cbf28f7
child 524566 aaa6e44260125781accde50de4db5aeefeb3011b
push id37323
push userdluca@mozilla.com
push dateFri, 17 Apr 2020 16:25:55 +0000
treeherdermozilla-central@b4b1d6f91ef0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1624266
milestone77.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 1624266 - Add code to page/worker/worklet code so that the global "SharedArrayBuffer" property can be trivially omitted from their global objects by changing how a single C++ variable for each case is initialized. r=baku Differential Revision: https://phabricator.services.mozilla.com/D71253
dom/base/nsGlobalWindowOuter.cpp
dom/media/webaudio/AudioWorkletGlobalScope.cpp
dom/workers/WorkerPrivate.cpp
layout/style/PaintWorkletGlobalScope.cpp
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -1999,36 +1999,44 @@ static JS::RealmCreationOptions& SelectZ
   return aOptions.setNewCompartmentAndZone();
 }
 
 /**
  * Create a new global object that will be used for an inner window.
  * Return the native global and an nsISupports 'holder' that can be used
  * to manage the lifetime of it.
  */
-static nsresult CreateNativeGlobalForInner(JSContext* aCx,
-                                           nsGlobalWindowInner* aNewInner,
-                                           nsIURI* aURI,
-                                           nsIPrincipal* aPrincipal,
-                                           JS::MutableHandle<JSObject*> aGlobal,
-                                           bool aIsSecureContext) {
+static nsresult CreateNativeGlobalForInner(
+    JSContext* aCx, nsGlobalWindowInner* aNewInner, nsIURI* aURI,
+    nsIPrincipal* aPrincipal, JS::MutableHandle<JSObject*> aGlobal,
+    bool aIsSecureContext, bool aDefineSharedArrayBufferConstructor) {
   MOZ_ASSERT(aCx);
   MOZ_ASSERT(aNewInner);
   MOZ_ASSERT(aPrincipal);
 
   // DOMWindow with nsEP is not supported, we have to make sure
   // no one creates one accidentally.
   nsCOMPtr<nsIExpandedPrincipal> nsEP = do_QueryInterface(aPrincipal);
   MOZ_RELEASE_ASSERT(!nsEP, "DOMWindow with nsEP is not supported");
 
   JS::RealmOptions options;
-
-  SelectZone(aCx, aPrincipal, aNewInner, options.creationOptions());
-
-  options.creationOptions().setSecureContext(aIsSecureContext);
+  JS::RealmCreationOptions& creationOptions = options.creationOptions();
+
+  SelectZone(aCx, aPrincipal, aNewInner, creationOptions);
+
+  creationOptions.setSecureContext(aIsSecureContext);
+
+  // Define the SharedArrayBuffer global constructor property only if shared
+  // memory may be used and structured-cloned (e.g. through postMessage).
+  //
+  // When the global constructor property isn't defined, the SharedArrayBuffer
+  // constructor can still be reached through Web Assembly.  Omitting the global
+  // property just prevents feature-tests from being misled.  See bug 1624266.
+  creationOptions.setDefineSharedArrayBufferConstructor(
+      aDefineSharedArrayBufferConstructor);
 
   xpc::InitGlobalObjectOptions(options, aPrincipal);
 
   // Determine if we need the Components object.
   bool needComponents = aPrincipal->IsSystemPrincipal();
   uint32_t flags = needComponents ? 0 : xpc::OMIT_COMPONENTS_OBJECT;
   flags |= xpc::DONT_FIRE_ONNEWGLOBALHOOK;
 
@@ -2204,22 +2212,38 @@ nsresult nsGlobalWindowOuter::SetNewDocu
       // a temporary global while initializing classes; the reason
       // being that xpconnect creates the temp global w/o a parent
       // and proto, which makes the JS engine look up classes in
       // cx->globalObject, i.e. this outer window].
 
       mInnerWindow = nullptr;
 
       mCreatingInnerWindow = true;
+
+      // The SharedArrayBuffer global constructor property should not be present
+      // in a fresh global object when shared memory objects aren't allowed
+      // (because COOP/COEP support isn't enabled, or because COOP/COEP don't
+      // act to isolate this page to a separate process).
+      //
+      // We set this value to |true| to replicate pre-existing behavior.  In the
+      // future, bug 1624266 will assign the correct COOP/COEP-respecting value
+      // here.  When that change is made, corresponding code for workers in
+      // WorkerPrivate.cpp must also be updated.  (Ideally both paint and audio
+      // worklets -- bug 1630876 and bug 1630877 -- would be fixed at the same
+      // time, but fixing them has lower priorit because they're not shipping
+      // yet.)
+      bool aDefineSharedArrayBufferConstructor = true;
+
       // Every script context we are initialized with must create a
       // new global.
       rv = CreateNativeGlobalForInner(
           cx, newInnerWindow, aDocument->GetDocumentURI(),
           aDocument->NodePrincipal(), &newInnerGlobal,
-          ComputeIsSecureContext(aDocument));
+          ComputeIsSecureContext(aDocument),
+          aDefineSharedArrayBufferConstructor);
       NS_ASSERTION(
           NS_SUCCEEDED(rv) && newInnerGlobal &&
               newInnerWindow->GetWrapperPreserveColor() == newInnerGlobal,
           "Failed to get script global");
 
       mCreatingInnerWindow = false;
       createdInnerWindow = true;
 
--- a/dom/media/webaudio/AudioWorkletGlobalScope.cpp
+++ b/dom/media/webaudio/AudioWorkletGlobalScope.cpp
@@ -36,16 +36,27 @@ NS_IMPL_RELEASE_INHERITED(AudioWorkletGl
 AudioWorkletGlobalScope::AudioWorkletGlobalScope(AudioWorkletImpl* aImpl)
     : WorkletGlobalScope(aImpl->GetAgentClusterId(),
                          aImpl->IsSharedMemoryAllowed()),
       mImpl(aImpl) {}
 
 bool AudioWorkletGlobalScope::WrapGlobalObject(
     JSContext* aCx, JS::MutableHandle<JSObject*> aReflector) {
   JS::RealmOptions options;
+
+  // The SharedArrayBuffer global constructor property should not be present in
+  // a fresh global object when shared memory objects aren't allowed (because
+  // COOP/COEP support isn't enabled, or because COOP/COEP don't act to isolate
+  // this worklet to a separate process).  However, it's not presently clear how
+  // to do this, so for now assign a backwards-compatible value.  Bug 1630877
+  // will fix this.
+  bool defineSharedArrayBufferConstructor = true;
+  options.creationOptions().setDefineSharedArrayBufferConstructor(
+      defineSharedArrayBufferConstructor);
+
   JS::AutoHoldPrincipals principals(aCx, new WorkletPrincipals(mImpl));
   return AudioWorkletGlobalScope_Binding::Wrap(
       aCx, this, this, options, principals.get(), true, aReflector);
 }
 
 void AudioWorkletGlobalScope::RegisterProcessor(
     JSContext* aCx, const nsAString& aName,
     AudioWorkletProcessorConstructor& aProcessorCtor, ErrorResult& aRv) {
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -2200,29 +2200,61 @@ WorkerPrivate::WorkerPrivate(
     if (aParent->mParentFrozen) {
       Freeze(nullptr);
     }
   } else {
     AssertIsOnMainThread();
 
     RuntimeService::GetDefaultJSSettings(mJSSettings);
 
-    mJSSettings.chrome.realmOptions.behaviors().setClampAndJitterTime(
-        !UsesSystemPrincipal());
-    mJSSettings.content.realmOptions.behaviors().setClampAndJitterTime(
-        !UsesSystemPrincipal());
-
-    mJSSettings.chrome.realmOptions.creationOptions().setToSourceEnabled(
-        UsesSystemPrincipal());
-    mJSSettings.content.realmOptions.creationOptions().setToSourceEnabled(
-        UsesSystemPrincipal());
-
-    if (mIsSecureContext) {
-      mJSSettings.chrome.realmOptions.creationOptions().setSecureContext(true);
-      mJSSettings.content.realmOptions.creationOptions().setSecureContext(true);
+    {
+      JS::RealmOptions& chromeRealmOptions = mJSSettings.chrome.realmOptions;
+      JS::RealmOptions& contentRealmOptions = mJSSettings.content.realmOptions;
+
+      JS::RealmBehaviors& chromeRealmBehaviors = chromeRealmOptions.behaviors();
+      JS::RealmBehaviors& contentRealmBehaviors =
+          contentRealmOptions.behaviors();
+
+      bool usesSystemPrincipal = UsesSystemPrincipal();
+
+      // Make timing imprecise in unprivileged code to blunt Spectre timing
+      // attacks.
+      bool clampAndJitterTime = !usesSystemPrincipal;
+      chromeRealmBehaviors.setClampAndJitterTime(clampAndJitterTime);
+      contentRealmBehaviors.setClampAndJitterTime(clampAndJitterTime);
+
+      JS::RealmCreationOptions& chromeCreationOptions =
+          chromeRealmOptions.creationOptions();
+      JS::RealmCreationOptions& contentCreationOptions =
+          contentRealmOptions.creationOptions();
+
+      // Expose uneval and toSource functions only if this is privileged code.
+      bool toSourceEnabled = usesSystemPrincipal;
+      chromeCreationOptions.setToSourceEnabled(toSourceEnabled);
+      contentCreationOptions.setToSourceEnabled(toSourceEnabled);
+
+      if (mIsSecureContext) {
+        chromeCreationOptions.setSecureContext(true);
+        contentCreationOptions.setSecureContext(true);
+      }
+
+      // The SharedArrayBuffer global constructor property should not be present
+      // in a fresh global object when shared memory objects aren't allowed
+      // (because COOP/COEP support isn't enabled, or because COOP/COEP don't
+      // act to isolate this worker to a separate process).
+      //
+      // Normal pages haven't yet been made to respect COOP/COEP in this regard
+      // yet -- they just always add the property.  This should be changed to
+      // |IsSharedMemoryAllowed()| when bug 1624266 fixes this for normal pages.
+      bool defineSharedArrayBufferConstructor = true;
+
+      chromeCreationOptions.setDefineSharedArrayBufferConstructor(
+          defineSharedArrayBufferConstructor);
+      contentCreationOptions.setDefineSharedArrayBufferConstructor(
+          defineSharedArrayBufferConstructor);
     }
 
     mIsInAutomation = xpc::IsInAutomation();
 
     // Our parent can get suspended after it initiates the async creation
     // of a new worker thread.  In this case suspend the new worker as well.
     if (mLoadInfo.mWindow && mLoadInfo.mWindow->IsSuspended()) {
       ParentWindowPaused();
--- a/layout/style/PaintWorkletGlobalScope.cpp
+++ b/layout/style/PaintWorkletGlobalScope.cpp
@@ -17,16 +17,27 @@ namespace dom {
 PaintWorkletGlobalScope::PaintWorkletGlobalScope(PaintWorkletImpl* aImpl)
     : WorkletGlobalScope(aImpl->GetAgentClusterId(),
                          aImpl->IsSharedMemoryAllowed()),
       mImpl(aImpl) {}
 
 bool PaintWorkletGlobalScope::WrapGlobalObject(
     JSContext* aCx, JS::MutableHandle<JSObject*> aReflector) {
   JS::RealmOptions options;
+
+  // The SharedArrayBuffer global constructor property should not be present in
+  // a fresh global object when shared memory objects aren't allowed (because
+  // COOP/COEP support isn't enabled, or because COOP/COEP don't act to isolate
+  // this worker to a separate process).  However, it's not presently clear how
+  // to do this, so for now assign a backwards-compatible value.  Bug 1630876
+  // will fix this.
+  bool defineSharedArrayBufferConstructor = true;
+  options.creationOptions().setDefineSharedArrayBufferConstructor(
+      defineSharedArrayBufferConstructor);
+
   JS::AutoHoldPrincipals principals(aCx, new WorkletPrincipals(mImpl));
   return PaintWorkletGlobalScope_Binding::Wrap(
       aCx, this, this, options, principals.get(), true, aReflector);
 }
 
 void PaintWorkletGlobalScope::RegisterPaint(const nsAString& aType,
                                             VoidFunction& aProcessorCtor) {
   // Nothing to do here, yet.