Skip to content

Commit 5148f62

Browse files
adetaylormibrunin
authored andcommitted
[Backport] Security bug 359992017
Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/6049756: Fix UaF in Linux accessibility code. Linux accessibility maintains a list of open menus; this list is used to compute the top level active frame by checking what frame owns any visible menu; it's also used for general checks about whether there's a menu open. It seems that in some circumstances requests arrive from the Linux accessibility APIs which confuse this list - the exact circumstances are not known, but it seems likely to be a race condition in when "menu open" and "menu close" messages arrive when there are UI interactions involving two menus. Unfortunately at the moment this may cause a use-after-free bug. This CL doesn't really attempt to resolve the underlying problem, but ensures we can't have a use-after-free bug in these circumstances, by ensuring we never keep a reference to a menu's ATK object. This may possibly confuse subsequent logic, but that seems preferable to a use-after-free bug, and in any case the list of menus seems like it should reflect the destruction of a menu. This problem has primarily been encountered during fuzzing, but has also been reported from unit testing and from users in the field. Bug: 359992017, 352530112, 40104941 Change-Id: I244270548df73898149b86d06e1853236d8ce835 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6049756 Reviewed-by: Greg Thompson <[email protected]> Commit-Queue: Adrian Taylor <[email protected]> Auto-Submit: Adrian Taylor <[email protected]> Cr-Commit-Position: refs/heads/main@{#1388727} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/624499 Reviewed-by: Anu Aliyas <[email protected]>
1 parent a1a8702 commit 5148f62

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

chromium/ui/accessibility/platform/ax_platform_node_auralinux.cc

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2561,6 +2561,11 @@ void AXPlatformNodeAuraLinux::DestroyAtkObjects() {
25612561
SetWeakGPtrToAtkObject(&g_current_focused, nullptr);
25622562
atk_object::Detach(AX_PLATFORM_NODE_AURALINUX(atk_object_));
25632563

2564+
// Under some circumstances, menu open and close requests can arrive
2565+
// in unexpected orders; let's ensure this object is no longer in
2566+
// the stack of open menus.
2567+
std::erase(GetActiveMenus(), atk_object_);
2568+
25642569
g_object_unref(atk_object_);
25652570
atk_object_ = nullptr;
25662571
}
@@ -3471,10 +3476,12 @@ void AXPlatformNodeAuraLinux::OnMenuPopupEnd() {
34713476
// kMenuPopupHide may be called multiple times for the same menu, so only
34723477
// remove it if our parent frame matches the most recently opened menu.
34733478
std::vector<AtkObject*>& active_menus = GetActiveMenus();
3474-
DCHECK(!active_menus.empty())
3475-
<< "Asymmetrical menupopupend events -- too many";
34763479

3477-
active_menus.pop_back();
3480+
// We don't trust menu show/hide events to arrive in
3481+
// any sensible order if multiple menus are involved, so ensure we delete
3482+
// this particular menu from the stack even if it's not topmost.
3483+
std::erase(active_menus, atk_object);
3484+
34783485
AtkObject* new_active_item = ComputeActiveTopLevelFrame();
34793486
if (new_active_item != parent_frame) {
34803487
// Newly activated menu has the different AtkWindow as the previous one.

0 commit comments

Comments
 (0)