From 66dd82116845addb08973d52e518db6e7ce5ff22 Mon Sep 17 00:00:00 2001

From: Milan Crha <mcrha@redhat.com>

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