From 9cbe4304facf8157e908a1fd05c045e6f5411950 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 4 Jul 2017 15:38:57 +0100 Subject: [PATCH 10/19] 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 --- dbus/dbus-message.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 234ddb23..119d8ac4 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2905,6 +2905,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter, DBusMessageRealIter *real_sub = (DBusMessageRealIter *)sub; DBusString contained_str; DBusValidity contained_signature_validity; + 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); @@ -2947,24 +2948,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; } -- 2.13.2