CVE-2024-10459

High CVSS 7.5 firefox UAF 🔧 Commit mapped

Overview

High

Severity

7.5

CVSS

Exploited ITW

Fixed

Fix Status

Impacthigh

DescriptionAn attacker could have caused a use-after-free when accessibility was enabled, leading to a potentially exploitable crash.

ComponentCore

Bug ClassUAF

Tracker1919087

Fix commit9fafa7208a2e (firefox) +79/-26

CWECWE-416 (Use-after-free)

CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H

CISA KEVNot listed

CreditedTyson Smith

Disclosed2024-10-29

Background

Accessibility relation: a link such as RelationType::LABEL_FOR or RelationType::DESCRIPTION_FOR that connects one LocalAccessible to another whose name or description it contributes to.
`LocalAccessible`: the in-process accessibility object mirroring a DOM node, whose lifetime is managed by the accessibility tree and its ref-counting.
Name/description change event: an AccEvent of type EVENT_NAME_CHANGE or EVENT_DESCRIPTION_CHANGE fired at the target whose accessible name or description was recomputed.
Mutation tick: the deferred phase in which NotificationController flushes queued tree mutations (EVENT_HIDE/EVENT_SHOW), during which the tree is reparented and relations are severed.

Root Cause Analysis

When a labelling or describing LocalAccessible was hidden or shown, the queued mutation event was processed later during the mutation tick, and only then did EventQueue::PushNameOrDescriptionChange walk the RelationType::LABEL_FOR and RelationType::DESCRIPTION_FOR relations to fire name/description change events on the related targets. By the time the tick ran, the tree had already been reparented and the relations severed, so the code walked stale relation state and dereferenced a LocalAccessible relation target that had been unparented and freed, producing a use-after-free.

The fix factors the relation walk into EventQueue::PushNameOrDescriptionChangeToRelations and calls it from NotificationController::QueueMutationEvent at queue time, while the relation is still intact, so the events are pushed against live objects before any unparenting occurs. It also hardens PushNameOrDescriptionChange to fire a name change when the recomputed name becomes void or falls back to a tooltip, covering names that were previously derived from a now-removed subtree.

Key insight

The core mistake was resolving accessibility relations too late — during the mutation tick, after the labelling node had already been unparented and its relation targets freed. The fix moves the relation resolution earlier, to EVENT_HIDE/EVENT_SHOW queue time in NotificationController::QueueMutationEvent, so events are pushed while the relations and their LocalAccessible targets are still valid.

Attack Path

Set up a label relation With accessibility enabled, craft a page where one element labels or describes another, establishing a RelationType::LABEL_FOR or RelationType::DESCRIPTION_FOR relation between two LocalAccessible objects.
Trigger a mutation Hide or remove the labelling element so an EVENT_HIDE (or reparent it for EVENT_SHOW) mutation event is queued in the NotificationController.
Deferred processing During the subsequent mutation tick, EventQueue::PushNameOrDescriptionChange runs, but the labelling accessible has already been unparented and its relation severed and freed.
Use-after-free The relation walk dereferences the now-freed LocalAccessible relation target while constructing an AccEvent, reading freed memory.
Exploit the crash Groom the freed allocation to control the reused memory, turning the dangling dereference into a potentially exploitable crash.

Impact Assessment

An attacker who lures a user with accessibility enabled to a malicious page gains a use-after-free in the browser’s main process, where the accessibility tree lives. Successful heap grooming of the freed LocalAccessible could allow controlled memory reads or corruption leading to code execution. The precondition is that accessibility is active (e.g. a screen reader or other assistive technology), which gates but does not eliminate the exposure.

Changed Functions

Function	Change	Notes
`if` accessible/base/EventQueue.cpp	modified
`switch` accessible/base/EventQueue.cpp	modified
`if` accessible/base/NotificationController.cpp	modified
`if` accessible/tests/mochitest/name/markup.js	modified

Files Changed

accessible/base/EventQueue.cpp
accessible/base/EventQueue.h
accessible/base/NotificationController.cpp
accessible/tests/mochitest/name/markup.js

Audit Directions

Deferred relation resolution
Look for accessibility code that walks Relation objects or relation targets during the mutation tick rather than at mutation queue time, since the tree may already be reparented and objects freed.
Lifetime across mutation phases
Audit any code holding raw LocalAccessible* pointers that spans an EVENT_HIDE/EVENT_SHOW boundary, where unparenting can free the referenced object before it is dereferenced.
Name/description recomputation
Review Name and description computations that assume a subtree still exists, checking for eNameOK void names, eNameFromTooltip fallbacks, and reorder events that silently change a container’s name.

Patch

view commit

diff --git a/accessible/base/EventQueue.cpp b/accessible/base/EventQueue.cpp
index 0b535029e7b..7cfc0fc94b3 100644
--- a/accessible/base/EventQueue.cpp
+++ b/accessible/base/EventQueue.cpp
@@ -51,6 +51,23 @@ bool EventQueue::PushEvent(AccEvent* aEvent) {
   return true;
 }
 
+bool EventQueue::PushNameOrDescriptionChangeToRelations(
+    LocalAccessible* aAccessible, RelationType aType) {
+  MOZ_ASSERT(aType == RelationType::LABEL_FOR || RelationType::DESCRIPTION_FOR);
+
+  bool pushed = false;
+  uint32_t eventType = aType == RelationType::LABEL_FOR
+                           ? nsIAccessibleEvent::EVENT_NAME_CHANGE
+                           : nsIAccessibleEvent::EVENT_DESCRIPTION_CHANGE;
+  Relation rel = aAccessible->RelationByType(aType);
+  while (LocalAccessible* relTarget = rel.LocalNext()) {
+    RefPtr<AccEvent> nameChangeEvent = new AccEvent(eventType, relTarget);
+    pushed |= PushEvent(nameChangeEvent);
+  }
+
+  return pushed;
+}
+
 bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
   // Fire name/description change event on parent or related LocalAccessible
   // being labelled/described given that this event hasn't been coalesced, the
@@ -58,14 +75,18 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
   // subtree was changed.
   LocalAccessible* target = aOrigEvent->mAccessible;
   // If the text of a text leaf changed without replacing the leaf, the only
-  // event we get is text inserted on the container. In this case, we might
-  // need to fire a name change event on the target itself.
+  // event we get is text inserted on the container. Or, a reorder event may
+  // change the container's name. In this case, we might need to fire a name
+  // change event on the target itself.
   const bool maybeTargetNameChanged =
       (aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_REMOVED ||
-       aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_INSERTED) &&
+       aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_TEXT_INSERTED ||
+       aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_REORDER ||
+       aOrigEvent->mEventType == nsIAccessibleEvent::EVENT_INNER_REORDER) &&
       nsTextEquivUtils::HasNameRule(target, eNameFromSubtreeRule);
   const bool doName = target->HasNameDependent() || maybeTargetNameChanged;
   const bool doDesc = target->HasDescriptionDependent();
+
   if (!doName && !doDesc) {
     return false;
   }
@@ -80,12 +101,34 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
     if (doName) {
       if (nameCheckAncestor && (maybeTargetNameChanged || parent != target) &&
           nsTextEquivUtils::HasNameRule(parent, eNameFromSubtreeRule)) {
-        nsAutoString name;
-        ENameValueFlag nameFlag = parent->Name(name);
-        // If name is obtained from subtree, fire name change event.
         // HTML file inputs always get part of their name from the subtree, even
         // if the author provided a name.
-        if (nameFlag == eNameFromSubtree || parent->IsHTMLFileInput()) {
+        bool fireNameChange = parent->IsHTMLFileInput();
+        if (!fireNameChange) {
+          nsAutoString name;
+          ENameValueFlag nameFlag = parent->Name(name);
+          switch (nameFlag) {
+            case eNameOK:
+              // Descendants of subtree may have been removed, making the name
+              // void.
+              fireNameChange = name.IsVoid();
+              break;
+            case eNameFromSubtree:
+              // If name is obtained from subtree, fire name change event.
+              fireNameChange = true;
+              break;
+            case eNameFromTooltip:
+              // If the descendants of this accessible were removed, the name
+              // may be calculated using the tooltip. We can guess that the name
+              // was obtained from the subtree before.
+              fireNameChange = true;
+              break;
+            default:
+              MOZ_ASSERT_UNREACHABLE("All name flags not covered!");
+          }
+        }
+
+        if (fireNameChange) {
           RefPtr<AccEvent> nameChangeEvent =
               new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, parent);
           pushed |= PushEvent(nameChangeEvent);
@@ -93,21 +136,13 @@ bool EventQueue::PushNameOrDescriptionChange(AccEvent* aOrigEvent) {
         nameCheckAncestor = false;
       }
 
-      Relation rel = parent->RelationByType(RelationType::LABEL_FOR);
-      while (LocalAccessible* relTarget = rel.LocalNext()) {
-        RefPtr<AccEvent> nameChangeEvent =
-            new AccEvent(nsIAccessibleEvent::EVENT_NAME_CHANGE, relTarget);
-        pushed |= PushEvent(nameChangeEvent);
-      }
+      pushed |= PushNameOrDescriptionChangeToRelations(parent,
+                                                       RelationType::LABEL_FOR);
     }
 
     if (doDesc) {
-      Relation rel = parent->RelationByType(RelationType::DESCRIPTION_FOR);
-      while (LocalAccessible* relTarget = rel.LocalNext()) {
-        RefPtr<AccEvent> descChangeEvent = new AccEvent(
-            nsIAccessibleEvent::EVENT_DESCRIPTION_CHANGE, relTarget);
-        pushed |= PushEvent(descChangeEvent);
-      }
+      pushed |= PushNameOrDescriptionChangeToRelations(
+          parent, RelationType::DESCRIPTION_FOR);
     }
 
     if (parent->IsDoc()) {
diff --git a/accessible/base/EventQueue.h b/accessible/base/EventQueue.h
index 87bcf893406..f2878eaf818 100644
--- a/accessible/base/EventQueue.h
+++ b/accessible/base/EventQueue.h
@@ -29,6 +29,9 @@ class EventQueue {
    */
   bool PushEvent(AccEvent* aEvent);
 
+  bool PushNameOrDescriptionChangeToRelations(LocalAccessible* aAccessible,
+                                              RelationType aType);
+
   /**
    * Puts name and/or description change events into the queue, if needed.
    */
diff --git a/accessible/base/NotificationController.cpp b/accessible/base/NotificationController.cpp
index d2c37fb5ad0..cd88ceb9a9e 100644
--- a/accessible/base/NotificationController.cpp
+++ b/accessible/base/NotificationController.cpp
@@ -203,6 +203,20 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
     }
   }
 
+  if (aEvent->GetEventType() == nsIAccessibleEvent::EVENT_HIDE ||
+      aEvent->GetEventType() == nsIAccessibleEvent::EVENT_SHOW) {
+    LocalAccessible* target = aEvent->GetAccessible();
+    // We need to do this here while the relation is still intact. During the
+    // tick, where we we call PushNameOrDescriptionChange, it will be too late
+    // since we will already have unparented the label and severed the relation.
+    if (PushNameOrDescriptionChangeToRelations(target,
+                                               RelationType::LABEL_FOR) ||
+        PushNameOrDescriptionChangeToRelations(target,
+                                               RelationType::DESCRIPTION_FOR)) {
+      ScheduleProcessing();
+    }
+  }
+
   // We need to fire a reorder event after all of the events targeted at shown
   // or hidden children of a container.  So either queue a new one, or move an
   // existing one to the end of the queue if the container already has a
@@ -213,12 +227,6 @@ bool NotificationController::QueueMutationEvent(AccTreeMutationEvent* aEvent) {
     reorder = new AccReorderEvent(container);
     container->SetReorderEventTarget(true);
     mMutationMap.PutEvent(reorder);
-
-    // Since this is the first child of container that is changing, the name
-    // and/or description of dependent Accessibles may be changing.
-    if (PushNameOrDescriptionChange(aEvent)) {
-      ScheduleProcessing();
-    }
   } else {
     AccReorderEvent* event = downcast_accEvent(
         mMutationMap.GetEvent(container, EventMap::ReorderEvent));
@@ -651,6 +659,13 @@ void NotificationController::ProcessMutationEvents() {
         return;
       }
 
+      // The mutation in the container can change its name, or an ancestor's
+      // name. A labelled/described by relation would also need to be notified
+      // if this is the case.
+      if (PushNameOrDescriptionChange(event)) {
+        ScheduleProcessing();
+      }
+
       LocalAccessible* target = event->GetAccessible();
       target->Document()->MaybeNotifyOfValueChange(target);
       if (!mDocument) {
diff --git a/accessible/tests/mochitest/name/markup.js b/accessible/tests/mochitest/name/markup.js
index 4ad478482b5..afe70fe9151 100644
--- a/accessible/tests/mochitest/name/markup.js
+++ b/accessible/tests/mochitest/name/markup.js
@@ -342,7 +342,7 @@ function testNameForSubtreeRule(aElm) {
 
   if (gDumpToConsole) {
     dump(
-      "\nProcessed from subtree rule. Wait for reorder event on " +
+      "\nProcessed from subtree rule. Wait for name change event on " +
         prettyName(aElm) +
         "\n"
     );

Loading diff…

Regression Test / PoC

shipped with the fix

diff --git a/accessible/tests/mochitest/name/markup.js b/accessible/tests/mochitest/name/markup.js
index 4ad478482b5..afe70fe9151 100644
--- a/accessible/tests/mochitest/name/markup.js
+++ b/accessible/tests/mochitest/name/markup.js
@@ -342,7 +342,7 @@ function testNameForSubtreeRule(aElm) {
 
   if (gDumpToConsole) {
     dump(
-      "\nProcessed from subtree rule. Wait for reorder event on " +
+      "\nProcessed from subtree rule. Wait for name change event on " +
         prettyName(aElm) +
         "\n"
     );

Loading diff…

References

Vendor advisory Fix commit Bug tracker NVD

On This Page