Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionInternal browser event interfaces were exposed to web content when privileged EventHandler listener callbacks ran for those events. Web content that tried to use those interfaces would not be able to use them with elevated privileges, but their presence would indicate certain browser features had been used, such as when a user opened the Dev Tools console.
ComponentSpiderMonkey
Bug ClassLogic Error
Tracker1906744
Fix commit47c9fe0b2b65 (firefox) +216/-250
CISA KEVNot listed
CreditedGregory Pappas
Disclosed2024-09-03

Changed Functions

FunctionChangeNotes
ProtoAndIfaceCache
dom/bindings/BindingDeclarations.h
modified
if
dom/bindings/BindingUtils.cpp
modified
CGCreateAndDefineOnGlobalMethod
dom/bindings/Codegen.py
modified

Files Changed

  • dom/bindings/BindingDeclarations.h
  • dom/bindings/BindingUtils.cpp
  • dom/bindings/BindingUtils.h
  • dom/bindings/Codegen.py
  • dom/bindings/WebIDLGlobalNameHash.cpp
  • dom/cache/CacheStorage.cpp
  • dom/cache/CacheStorage.h
  • dom/indexedDB/IndexedDatabaseManager.cpp
  • dom/workers/RegisterBindings.cpp
  • extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp
  • js/xpconnect/src/Sandbox.cpp
diff --git a/dom/bindings/BindingDeclarations.h b/dom/bindings/BindingDeclarations.h
index c723af00212..83919e7e5a9 100644
--- a/dom/bindings/BindingDeclarations.h
+++ b/dom/bindings/BindingDeclarations.h
@@ -548,14 +548,33 @@ class SystemCallerGuarantee {
   operator CallerType() const { return CallerType::System; }
 };
 
+enum class DefineInterfaceProperty {
+  No,
+  CheckExposure,
+  Always,
+};
+
 class ProtoAndIfaceCache;
-typedef void (*CreateInterfaceObjectsMethod)(JSContext* aCx,
-                                             JS::Handle<JSObject*> aGlobal,
-                                             ProtoAndIfaceCache& aCache,
-                                             bool aDefineOnGlobal);
+using CreateInterfaceObjectsMethod =
+    void (*)(JSContext*, JS::Handle<JSObject*>, ProtoAndIfaceCache&,
+             DefineInterfaceProperty aDefineOnGlobal);
+
+// GetPerInterfaceObjectHandle has 3 possible behaviours for defining the named
+// properties on the global for an interface or namespace when it creates an
+// interface or namespace object. aDefineOnGlobal can be used to pick the
+// behaviour. GetPerInterfaceObjectHandle either:
+//
+//  * does not define any properties on the global object
+//    (for DefineInterfaceProperty::No),
+//  * checks whether the interface is exposed in the global object before
+//    defining properties (for DefineInterfaceProperty::CheckExposure),
+//  * always defines properties (for DefineInterfaceProperty::Always).
+//
+// Callers should be careful when passing DefineInterfaceProperty::Always and
+// make sure to check exposure themselves if needed.
 JS::Handle<JSObject*> GetPerInterfaceObjectHandle(
     JSContext* aCx, size_t aSlotId, CreateInterfaceObjectsMethod aCreator,
-    bool aDefineOnGlobal);
+    DefineInterfaceProperty aDefineOnGlobal);
 
 namespace binding_detail {
 
diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp
index 017b0bc322d..b64c49cde76 100644
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -3665,8 +3665,8 @@ bool GetDesiredProto(JSContext* aCx, const JS::CallArgs& aCallArgs,
     // JS::GetRealmGlobalOrNull should not be returning null here, because we
     // have live objects in the Realm.
     JSAutoRealm ar(aCx, JS::GetRealmGlobalOrNull(realm));
-    aDesiredProto.set(
-        GetPerInterfaceObjectHandle(aCx, aProtoId, aCreator, true));
+    aDesiredProto.set(GetPerInterfaceObjectHandle(
+        aCx, aProtoId, aCreator, DefineInterfaceProperty::CheckExposure));
     if (!aDesiredProto) {
       return false;
     }
@@ -3797,8 +3797,8 @@ bool HTMLConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp,
   // makes sense to start with: https://github.com/whatwg/html/issues/3575
   {
     JSAutoRealm ar(aCx, newTarget);
-    JS::Handle<JSObject*> constructor =
-        GetPerInterfaceObjectHandle(aCx, aConstructorId, aCreator, true);
+    JS::Handle<JSObject*> constructor = GetPerInterfaceObjectHandle(
+        aCx, aConstructorId, aCreator, DefineInterfaceProperty::CheckExposure);
     if (!constructor) {
       return false;
     }
@@ -4205,7 +4205,7 @@ JSObject* UnprivilegedJunkScopeOrWorkerGlobal(const fallible_t&) {
 
 JS::Handle<JSObject*> GetPerInterfaceObjectHandle(
     JSContext* aCx, size_t aSlotId, CreateInterfaceObjectsMethod aCreator,
-    bool aDefineOnGlobal) {
+    DefineInterfaceProperty aDefineOnGlobal) {
   /* Make sure our global is sane.  Hopefully we can remove this sometime */
   JSObject* global = JS::CurrentGlobalOrNull(aCx);
   if (!(JS::GetClass(global)->flags & JSCLASS_DOM_GLOBAL)) {
diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h
index 6bcefbf3158..49c23890f1e 100644
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -3369,6 +3369,14 @@ class StringIdChars {
 already_AddRefed<Promise> CreateRejectedPromiseFromThrownException(
     JSContext* aCx, ErrorResult& aError);
 
+template <auto ConstructorEnabled>
+inline bool ShouldExpose(JSContext* aCx, JS::Handle<JSObject*> aGlobal,
+                         DefineInterfaceProperty aDefine) {
+  return aDefine == DefineInterfaceProperty::Always ||
+         (aDefine == DefineInterfaceProperty::CheckExposure &&
+          ConstructorEnabled(aCx, aGlobal));
+}
+
 }  // namespace binding_detail
 
 }  // namespace dom
diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py
index 659d7e4b200..d70f9d528d7 100644
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -3494,7 +3494,7 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
             Argument("JSContext*", "aCx"),
             Argument("JS::Handle<JSObject*>", "aGlobal"),
             Argument("ProtoAndIfaceCache&", "aProtoAndIfaceCache"),
-            Argument("bool", "aDefineOnGlobal"),
+            Argument("DefineInterfaceProperty", "aDefineOnGlobal"),
         ]
         CGAbstractMethod.__init__(
             self, descriptor, "CreateInterfaceObjects", "void", args, static=static
@@ -3505,6 +3505,19 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
 
     def definition_body(self):
         needInterfaceObject = self.descriptor.interface.hasInterfaceObject()
+        if needInterfaceObject and self.descriptor.isExposedConditionally():
+            # This code might be called when we're trying to create an object
+            # in a non-system compartment, for example when system code is
+            # calling a constructor through Xrays. In that case we do want to
+            # create an interface object in the non-system compartment, but we
+            # don't want to expose the name on the non-system global if the
+            # interface itself is marked as ChromeOnly.
+            defineOnGlobal = (
+                "ShouldExpose<%s::ConstructorEnabled>(aCx, aGlobal, aDefineOnGlobal)"
+                % toBindingNamespace(self.descriptor.name)
+            )
+        else:
+            defineOnGlobal = "aDefineOnGlobal != DefineInterfaceProperty::No"
         if needInterfaceObject:
             (protoGetter, protoHandleGetter) = InterfaceObjectProtoGetter(
                 self.descriptor
@@ -3572,13 +3585,15 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
                                            interfaceCache,
                                            ${properties},
                                            ${chromeProperties},
-                                           "${name}", aDefineOnGlobal);
+                                           "${name}",
+                                           ${defineOnGlobal});
                 """,
                 interfaceCache=interfaceCache,
                 constructorProto=constructorProto,
                 properties=properties,
                 chromeProperties=chromeProperties,
                 name=name,
+                defineOnGlobal=defineOnGlobal,
             )
             return CGList(
                 [
@@ -3659,7 +3674,8 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
                                         interfaceCache,
                                         ${properties},
                                         ${chromeProperties},
-                                        "${name}", aDefineOnGlobal,
+                                        "${name}",
+                                        ${defineOnGlobal},
                                         ${unscopableNames},
                                         ${isGlobal},
                                         ${legacyWindowAliases});
@@ -3674,6 +3690,7 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
             properties=properties,
             chromeProperties=chromeProperties,
             name=name,
+            defineOnGlobal=defineOnGlobal,
             unscopableNames="unscopableNames" if self.haveUnscopables else "nullptr",
             isGlobal=toStringBool(isGlobal),
             legacyWindowAliases="legacyWindowAliases"
@@ -3913,6 +3930,40 @@ class CGCreateInterfaceObjectsMethod(CGAbstractMethod):
         ).define()
 
 
+class CGCreateAndDefineOnGlobalMethod(CGAbstractMethod):
+    """
+    A method for creating the interface or namespace object and defining
+    properties for it on the global.
+    """
+
+    def __init__(self, descriptor):
+        CGAbstractMethod.__init__(
+            self,
+            descriptor,
+            "CreateAndDefineOnGlobal",
+            "bool",
+            [
+                Argument("JSContext*", "aCx"),
+            ],
+            inline=True,
+        )
+
+    def definition_body(self):
+        return fill(
+            """
+            // Get the interface or namespace object for this class. This will
+            // create the object as needed and always define the properties for
+            // it on the global. The caller should make sure the interface or
+            // namespace is exposed on the global before calling this.
+            return GetPerInterfaceObjectHandle(aCx, constructors::id::${name},
+                                               &CreateInterfaceObjects,
+                                               DefineInterfaceProperty::Always);
+
+            """,
+            name=self.descriptor.name,
Loading diff…