Firefox · Core
CVE-2024-10459
UAF in Core
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifaccessible/base/EventQueue.cpp |
modified | |
switchaccessible/base/EventQueue.cpp |
modified | |
ifaccessible/base/NotificationController.cpp |
modified | |
ifaccessible/tests/mochitest/name/markup.js |
modified |
Files Changed
accessible/base/EventQueue.cppaccessible/base/EventQueue.haccessible/base/NotificationController.cppaccessible/tests/mochitest/name/markup.js
Patch
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
On This Page