Medium CVSS 8.8 firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
8.8
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
CWECWE-273 (Improper check for dropped privileges)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
CISA KEVNot listed
CreditedGregory Pappas
Disclosed2024-09-03

Background

`GetPerInterfaceObjectHandle`
The dom binding helper that lazily creates a WebIDL interface/prototype object for a given slot and optionally defines its named property on the current global.
`ProtoAndIfaceCache`
Per-realm cache that stores the prototype and interface objects for every WebIDL interface, populated by generated CreateInterfaceObjects methods.
Xray wrappers
Mechanism by which privileged (system) code reaches into a less-privileged compartment’s objects, which can trigger interface-object creation inside that non-system compartment.
`ConstructorEnabled`
A generated per-interface predicate that reports whether an interface is exposed in a given global, honoring ChromeOnly and other exposure conditions.

Root Cause Analysis

When system code interacted with a web-content compartment through Xrays — such as when a privileged EventHandler listener callback ran for an internal browser event — it could cause GetPerInterfaceObjectHandle to create an interface object in the non-system (web content) compartment. The old signature took a plain bool aDefineOnGlobal, and callers like GetDesiredProto and HTMLConstructor passed true, which caused the generated CreateInterfaceObjects method to unconditionally define the interface’s named property on the non-system global without ever consulting the interface’s exposure conditions. This violated the invariant that ChromeOnly/conditionally-exposed interfaces must never have their names defined on a web-content global, so internal browser interfaces leaked into web content.

The fix replaces the boolean with the tri-state DefineInterfaceProperty enum (No, CheckExposure, Always) and introduces the ShouldExpose helper, so that for conditionally-exposed interfaces the generated code calls ShouldExpose<...::ConstructorEnabled> and only defines the name when the interface is actually exposed in that global. This works because exposure is now re-checked at definition time in the target compartment rather than being implied by a true flag chosen by the caller.

Key insight
The single core mistake was conflating “create the interface object” with “define its name on the global,” letting a caller-supplied bool aDefineOnGlobal = true bypass per-global exposure checks; the fix separates those concerns via the DefineInterfaceProperty enum and the ShouldExpose predicate so that CheckExposure re-validates ConstructorEnabled against the actual (possibly non-system) global before defining the name.

Attack Path

  1. Trigger privileged event handling Cause a privileged EventHandler listener to run for an internal browser event (for example, opening the Dev Tools console) so that system code touches a web-content compartment.
  2. Cross into content via Xrays The system code reaches into the non-system compartment through Xray wrappers, invoking a constructor path such as HTMLConstructor or GetDesiredProto.
  3. Force interface-object creation These callers invoke GetPerInterfaceObjectHandle with aDefineOnGlobal = true, driving the generated CreateInterfaceObjects for the interface in the content compartment.
  4. Leak the interface name Because the boolean unconditionally defined the property, the ChromeOnly interface’s name became visible on the web-content global even though its exposure condition was not met.
  5. Observe from content Web content enumerates or probes the global and detects the presence of the internal interface, inferring that a privileged browser feature was used.

Impact Assessment

An attacker running in a web-content process gains an information-disclosure primitive: the presence of internal ChromeOnly interface names on the content global reveals that specific browser features (such as the Dev Tools console) were exercised. The exposed interfaces cannot be used with elevated privileges from content, so the gain is limited to observing browser state rather than executing privileged operations. The precondition is that privileged code must run through Xrays against the content compartment for an affected event, so the leak is opportunistic rather than directly controllable.

Changed Functions

FunctionChangeNotes
ProtoAndIfaceCache
dom/bindings/BindingDeclarations.h
modified
if
dom/bindings/BindingUtils.cpp
modified
CGCreateAndDefineOnGlobalMethod
dom/bindings/Codegen.py
modified
CGGetProtoObjectHandleMethod
dom/bindings/Codegen.py
modified
CGRegisterWorkerBindings
dom/bindings/Codegen.py
modified
CGRegisterWorkerDebuggerBindings
dom/bindings/Codegen.py
modified
CGRegisterWorkletBindings
dom/bindings/Codegen.py
modified
CGRegisterShadowRealmBindings
dom/bindings/Codegen.py
modified
if
dom/bindings/WebIDLGlobalNameHash.cpp
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

Audit Directions

  • Boolean exposure flags
    Search for any remaining bool aDefineOnGlobal-style parameters or callers passing literal true/false for interface exposure, since a hard-coded boolean can bypass per-global ConstructorEnabled checks.
  • Xray-driven object creation
    Review code paths where system code creates or fetches WebIDL objects inside non-system compartments (constructors, proto lookups) and verify DefineInterfaceProperty::CheckExposure is used rather than Always.
  • `ChromeOnly` conditional interfaces
    Audit interfaces marked ChromeOnly or otherwise conditionally exposed to confirm their generated CreateInterfaceObjects routes name definition through ShouldExpose and never defines names unconditionally on content globals.
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,
+        )
+
+
 class CGGetProtoObjectHandleMethod(CGAbstractMethod):
     """
     A method for getting the interface prototype object.
@@ -3937,7 +3988,7 @@ class CGGetProtoObjectHandleMethod(CGAbstractMethod):
                object as needed. */
             return GetPerInterfaceObjectHandle(aCx, prototypes::id::${name},
                                                &CreateInterfaceObjects,
-                                               /* aDefineOnGlobal = */ true);
+                                               DefineInterfaceProperty::CheckExposure);
 
             """,
             name=self.descriptor.name,
@@ -3975,7 +4026,6 @@ class CGGetConstructorObjectHandleMethod(CGAbstractMethod):
             "JS::Handle<JSObject*>",
             [
                 Argument("JSContext*", "aCx"),
-                Argument("bool", "aDefineOnGlobal", "true"),
             ],
             inline=True,
         )
@@ -3988,7 +4038,7 @@ class CGGetConstructorObjectHandleMethod(CGAbstractMethod):
 
             return GetPerInterfaceObjectHandle(aCx, constructors::id::${name},
                                                &CreateInterfaceObjects,
-                                               aDefineOnGlobal);
+                                               DefineInterfaceProperty::CheckExposure);
             """,
             name=self.descriptor.name,
         )
@@ -17172,6 +17222,11 @@ class CGDescriptor(CGThing):
         if descriptor.interface.hasInterfaceObject():
             cgThings.append(CGGetConstructorObjectHandleMethod(descriptor))
             cgThings.append(CGGetConstructorObjectMethod(descriptor))
+            cgThings.append(
+                CGCreateAndDefineOnGlobalMethod(
+                    descriptor,
+                )
+            )
 
         # See whether we need to generate cross-origin property arrays.
         if needCrossOriginPropertyArrays:
@@ -18355,6 +18410,21 @@ class CGDictionary(CGThing):
         return all(CGDictionary.typeSafeToJSONify(m.type) for m in dictionary.members)
 
 
+def RegisterNonWindowBindings(descriptors):
+    conditions = []
+    for desc in descriptors:
+        bindingNS = toBindingNamespace(desc.name)
+        condition = "!%s::CreateAndDefineOnGlobal(aCx)" % bindingNS
+        if desc.isExposedConditionally():
+            condition = "%s::ConstructorEnabled(aCx, aObj) && " % bindingNS + condition
+        conditions.append(condition)
+    lines = [
+        CGIfWrapper(CGGeneric("return false;\n"), condition) for condition in conditions
+    ]
+    lines.append(CGGeneric("return true;\n"))
+    return CGList(lines, "\n").define()
+
+
 class CGRegisterWorkerBindings(CGAbstractMethod):
     def __init__(self, config):
         CGAbstractMethod.__init__(
@@ -18367,24 +18437,11 @@ class CGRegisterWorkerBindings(CGAbstractMethod):
         self.config = config
 
     def definition_body(self):
-        descriptors = self.config.getDescriptors(
-            hasInterfaceObject=True, isExposedInAnyWorker=True, register=True
+        return RegisterNonWindowBindings(
+            self.config.getDescriptors(
+                hasInterfaceObject=True, isExposedInAnyWorker=True, register=True
+            )
         )
-        conditions = []
-        for desc in descriptors:
-            bindingNS = toBindingNamespace(desc.name)
-            condition = "!%s::GetConstructorObject(aCx)" % bindingNS
-            if desc.isExposedConditionally():
-                condition = (
-                    "%s::ConstructorEnabled(aCx, aObj) && " % bindingNS + condition
-                )
-            conditions.append(condition)
-        lines = [
-            CGIfWrapper(CGGeneric("return false;\n"), condition)
-            for condition in conditions
-        ]
-        lines.append(CGGeneric("return true;\n"))
-        return CGList(lines, "\n").define()
 
 
 class CGRegisterWorkerDebuggerBindings(CGAbstractMethod):
@@ -18399,24 +18456,11 @@ class CGRegisterWorkerDebuggerBindings(CGAbstractMethod):
         self.config = config
 
     def definition_body(self):
-        descriptors = self.config.getDescriptors(
-            hasInterfaceObject=True, isExposedInWorkerDebugger=True, register=True
+        return RegisterNonWindowBindings(
+            self.config.getDescriptors(
+                hasInterfaceObject=True, isExposedInWorkerDebugger=True, register=True
+            )
         )
-        conditions = []
-        for desc in descriptors:
-            bindingNS = toBindingNamespace(desc.name)
-            condition = "!%s::GetConstructorObject(aCx)" % bindingNS
-            if desc.isExposedConditionally():
-                condition = (
-                    "%s::ConstructorEnabled(aCx, aObj) && " % bindingNS + condition
-                )
-            conditions.append(condition)
-        lines = [
-            CGIfWrapper(CGGeneric("return false;\n"), condition)
-            for condition in conditions
-        ]
-        lines.append(CGGeneric("return true;\n"))
-        return CGList(lines, "\n").define()
 
 
 class CGRegisterWorkletBindings(CGAbstractMethod):
@@ -18431,24 +18475,11 @@ class CGRegisterWorkletBindings(CGAbstractMethod):
         self.config = config
 
     def definition_body(self):
-        descriptors = self.config.getDescriptors(
-            hasInterfaceObject=True, isExposedInAnyWorklet=True, register=True
+        return RegisterNonWindowBindings(
+            self.config.getDescriptors(
+                hasInterfaceObject=True, isExposedInAnyWorklet=True, register=True
+            )
         )
-        conditions = []
-        for desc in descriptors:
-            bindingNS = toBindingNamespace(desc.name)
-            condition = "!%s::GetConstructorObject(aCx)" % bindingNS
-            if desc.isExposedConditionally():
-                condition = (
-                    "%s::ConstructorEnabled(aCx, aObj) && " % bindingNS + condition
-                )
-            conditions.append(condition)
-        lines = [
-            CGIfWrapper(CGGeneric("return false;\n"), condition)
-            for condition in conditions
-        ]
-        lines.append(CGGeneric("return true;\n"))
-        return CGList(lines, "\n").define()
 
 
 class CGRegisterShadowRealmBindings(CGAbstractMethod):
@@ -18463,24 +18494,11 @@ class CGRegisterShadowRealmBindings(CGAbstractMethod):
         self.config = config
 
     def definition_body(self):
-        descriptors = self.config.getDescriptors(
-            hasInterfaceObject=True, isExposedInShadowRealms=True, register=True
+        return RegisterNonWindowBindings(
+            self.config.getDescriptors(
+                hasInterfaceObject=True, isExposedInShadowRealms=True, register=True
+            )
         )
-        conditions = []
-        for desc in descriptors:
-            bindingNS = toBindingNamespace(desc.name)
-            condition = "!%s::GetConstructorObject(aCx)" % bindingNS
-            if desc.isExposedConditionally():
-                condition = (
-                    "%s::ConstructorEnabled(aCx, aObj) && " % bindingNS + condition
-                )
-            conditions.append(condition)
-        lines = [
-            CGIfWrapper(CGGeneric("return false;\n"), condition)
-            for condition in conditions
-        ]
-        lines.append(CGGeneric("return true;\n"))
-        return CGList(lines, "\n").define()
 
 
 def BindingNamesOffsetEnum(name):
diff --git a/dom/bindings/WebIDLGlobalNameHash.cpp b/dom/bindings/WebIDLGlobalNameHash.cpp
index 24e48a343fc..9e87c895367 100644
--- a/dom/bindings/WebIDLGlobalNameHash.cpp
+++ b/dom/bindings/WebIDLGlobalNameHash.cpp
@@ -33,7 +33,7 @@ static JSObject* FindNamedConstructorForXray(
     JSContext* aCx, JS::Handle<jsid> aId, const WebIDLNameTableEntry* aEntry) {
   JSObject* interfaceObject =
       GetPerInterfaceObjectHandle(aCx, aEntry->mConstructorId, aEntry->mCreate,
-                                  /* aDefineOnGlobal = */ false);
+                                  DefineInterfaceProperty::No);
   if (!interfaceObject) {
     return nullptr;
   }
@@ -161,10 +161,13 @@ bool WebIDLGlobalNameHash::DefineIfEnabled(
     return true;
   }
 
+  // We've already checked whether the interface is enabled (see
... (file diff truncated)
Loading diff…