From 66dd82116845addb08973d52e518db6e7ce5ff22 Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Mon, 8 May 2017 17:21:58 -0500 Subject: [PATCH] Fix use after free when returned objects hold only one ref It seems that not all code expects atk_object_ref_accessible_child() returning NULL, neither that it can return an object with only one reference, thus the following unref in the code can cause use-after-free eventually. At least the chunk in impl_GetChildAtIndex() avoids runtime warning about invalid object being passed to g_object_unref(), which happened, in this case, when evolution returned NULL. Evolution returns objects with one reference only often, which tries to address the other chunks here. https://bugzilla.gnome.org/show_bug.cgi?id=781716 --- atk-adaptor/adaptors/accessible-adaptor.c | 3 ++- atk-adaptor/adaptors/collection-adaptor.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/atk-adaptor/adaptors/accessible-adaptor.c b/atk-adaptor/adaptors/accessible-adaptor.c index 058b116..572e4f8 100644 --- a/atk-adaptor/adaptors/accessible-adaptor.c +++ b/atk-adaptor/adaptors/accessible-adaptor.c @@ -182,7 +182,8 @@ impl_GetChildAtIndex (DBusConnection * bus, } child = atk_object_ref_accessible_child (object, i); reply = spi_object_return_reference (message, child); - g_object_unref (child); + if (child) + g_object_unref (child); return reply; } diff --git a/atk-adaptor/adaptors/collection-adaptor.c b/atk-adaptor/adaptors/collection-adaptor.c index 42ea073..b57c5f6 100644 --- a/atk-adaptor/adaptors/collection-adaptor.c +++ b/atk-adaptor/adaptors/collection-adaptor.c @@ -494,9 +494,12 @@ sort_order_canonical (MatchRulePrivate * mrp, GList * ls, { AtkObject *child = atk_object_ref_accessible_child (obj, i); - g_object_unref (child); + if (!child) + continue; + if (prev && child == pobj) { + g_object_unref (child); return kount; } @@ -517,6 +520,7 @@ sort_order_canonical (MatchRulePrivate * mrp, GList * ls, kount = sort_order_canonical (mrp, ls, kount, max, child, 0, TRUE, pobj, recurse, traverse); + g_object_unref (child); } return kount; } @@ -559,19 +563,23 @@ sort_order_rev_canonical (MatchRulePrivate * mrp, GList * ls, and get it's last descendant. First, get the previous sibling */ nextobj = atk_object_ref_accessible_child (parent, indexinparent - 1); - g_object_unref (nextobj); /* Now, drill down the right side to the last descendant */ - while (atk_object_get_n_accessible_children (nextobj) > 0) + while (nextobj && atk_object_get_n_accessible_children (nextobj) > 0) { - nextobj = atk_object_ref_accessible_child (nextobj, + AtkObject *follow; + + follow = atk_object_ref_accessible_child (nextobj, atk_object_get_n_accessible_children (nextobj) - 1); g_object_unref (nextobj); + nextobj = follow; } /* recurse with the last descendant */ kount = sort_order_rev_canonical (mrp, ls, kount, max, nextobj, TRUE, pobj); + if (nextobj) + g_object_unref (nextobj); } else if (max == 0 || kount < max) { -- 2.9.3