From cd5f9ef840f34563f38df81620bc5fa82bf7df5c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 4 Jul 2017 16:04:07 +0100 Subject: [PATCH 14/19] DBusMessageIter: Zero out the iterator on failure This ensures that callers won't accidentally use it for something in a way that is considered to be programmer error. In _dbus_message_iter_check(), insert a specific check for this before dereferencing iter->message, so that we get a nice assertion failure (potentially non-fatal) instead of a segfault. Signed-off-by: Simon McVittie --- dbus/dbus-message.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index adde30bd..dc7f0e59 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -709,6 +709,19 @@ dbus_message_cache_or_finalize (DBusMessage *message) dbus_message_finalize (message); } +/* + * Arrange for iter to be something that _dbus_message_iter_check() would + * reject as not a valid iterator. + */ +static void +_dbus_message_real_iter_zero (DBusMessageRealIter *iter) +{ + _dbus_assert (iter != NULL); + _DBUS_ZERO (*iter); + /* NULL is not, strictly speaking, guaranteed to be all-bits-zero */ + iter->message = NULL; +} + #if defined(DBUS_ENABLE_CHECKS) || defined(DBUS_ENABLE_ASSERT) static dbus_bool_t _dbus_message_iter_check (DBusMessageRealIter *iter) @@ -721,6 +734,13 @@ _dbus_message_iter_check (DBusMessageRealIter *iter) return FALSE; } + if (iter->message == NULL || iter->iter_type == 0) + { + _dbus_warn_check_failed ("dbus message iterator has already been " + "closed, or is uninitialized or corrupt"); + return FALSE; + } + byte_order = _dbus_header_get_byte_order (&iter->message->header); if (iter->iter_type == DBUS_MESSAGE_ITER_TYPE_READER) @@ -2911,10 +2931,14 @@ dbus_message_iter_open_container (DBusMessageIter *iter, DBusValidity contained_signature_validity; dbus_bool_t ret; + _dbus_return_val_if_fail (sub != NULL, FALSE); + /* Do our best to make sure the sub-iterator doesn't contain something + * valid-looking on failure */ + _dbus_message_real_iter_zero (real_sub); + _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); _dbus_return_val_if_fail (dbus_type_is_container (type), FALSE); - _dbus_return_val_if_fail (sub != NULL, FALSE); _dbus_return_val_if_fail ((type == DBUS_TYPE_STRUCT && contained_signature == NULL) || (type == DBUS_TYPE_DICT_ENTRY && @@ -3011,6 +3035,7 @@ dbus_message_iter_close_container (DBusMessageIter *iter, ret = _dbus_type_writer_unrecurse (&real->u.writer, &real_sub->u.writer); + _dbus_message_real_iter_zero (real_sub); if (!_dbus_message_iter_close_signature (real)) ret = FALSE; @@ -3034,9 +3059,9 @@ dbus_message_iter_abandon_container (DBusMessageIter *iter, DBusMessageIter *sub) { DBusMessageRealIter *real = (DBusMessageRealIter *)iter; -#ifndef DBUS_DISABLE_CHECKS DBusMessageRealIter *real_sub = (DBusMessageRealIter *)sub; +#ifndef DBUS_DISABLE_CHECKS _dbus_return_if_fail (_dbus_message_iter_append_check (real)); _dbus_return_if_fail (real->iter_type == DBUS_MESSAGE_ITER_TYPE_WRITER); _dbus_return_if_fail (_dbus_message_iter_append_check (real_sub)); @@ -3044,6 +3069,7 @@ dbus_message_iter_abandon_container (DBusMessageIter *iter, #endif _dbus_message_iter_abandon_signature (real); + _dbus_message_real_iter_zero (real_sub); } /** -- 2.13.2