From 4633187bb24e9b1219e9127214ca4c0c550803eb Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 4 Jul 2017 15:38:57 +0100 Subject: [PATCH] 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. Change-Id: I2ccd4b516c7714f64c4543dd8d2e5c99633733a5 Signed-off-by: Simon McVittie Reviewed-by: Philip Withnall Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568 --- dbus/dbus-message.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 18388a1..93ffb40 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -3047,6 +3047,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); @@ -3073,24 +3074,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, + 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, + 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.7.4