From: Simon McVittie <smcv@collabora.com>
Date: Tue, 4 Jul 2017 14:38:57 +0000 (+0100)
Subject: dbus_message_iter_open_container: Don't leak signature on failure
X-Git-Tag: dbus-1.12.0~47^2~18
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c462a6b395cf1e46df72c4d6edd5c5e401e79863;p=platform%2Fupstream%2Fdbus.git

dbus_message_iter_open_container: Don't leak signature on failure

If we run out of memory while calling _dbus_type_writer_recurse()
(which is impossible for most contained types, but can happen for
structs and dict-entries), then the memory we allocated in the call to
_dbus_message_iter_open_signature() will still be allocated, and we
have to free it in order to return to the state of the world prior to
calling open_container().

One might reasonably worry that this change can break callers that use
this (incorrect) pattern:

    if (!dbus_message_iter_open_container (outer, ..., inner))
      {
        dbus_message_iter_abandon_container (outer, inner);
        goto fail;
      }
    /* now we know inner is open, and we must close it later */

However, testing that pattern with _dbus_test_oom_handling()
demonstrates that it already dies with a DBusString assertion failure
even before this commit.

This is all concerningly fragile, and I think the next step should be
to zero out DBusMessageIter instances when they are invalidated, so
that a "double-free" is always detected.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 031aa2ceb3dfff373e7b398dfc5d020d77262512)
---

diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 7186e6d..4f32fd8 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -2938,6 +2938,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
   DBusMessageRealIter *real = (DBusMessageRealIter *)iter;
   DBusMessageRealIter *real_sub = (DBusMessageRealIter *)sub;
   DBusString contained_str;
+  dbus_bool_t ret;
 
   _dbus_return_val_if_fail (_dbus_message_iter_append_check (real), FALSE);
   _dbus_return_val_if_fail (real->iter_type == DBUS_MESSAGE_ITER_TYPE_WRITER, FALSE);
@@ -2964,24 +2965,30 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
   if (!_dbus_message_iter_open_signature (real))
     return FALSE;
 
+  ret = FALSE;
   *real_sub = *real;
 
   if (contained_signature != NULL)
     {
       _dbus_string_init_const (&contained_str, contained_signature);
 
-      return _dbus_type_writer_recurse (&real->u.writer,
-                                        type,
-                                        &contained_str, 0,
-                                        &real_sub->u.writer);
+      ret = _dbus_type_writer_recurse (&real->u.writer,
+                                       type,
+                                       &contained_str, 0,
+                                       &real_sub->u.writer);
     }
   else
     {
-      return _dbus_type_writer_recurse (&real->u.writer,
-                                        type,
-                                        NULL, 0,
-                                        &real_sub->u.writer);
-    } 
+      ret = _dbus_type_writer_recurse (&real->u.writer,
+                                       type,
+                                       NULL, 0,
+                                       &real_sub->u.writer);
+    }
+
+  if (!ret)
+    _dbus_message_iter_abandon_signature (real);
+
+  return ret;
 }