From 0e53d4eed36f378e99802e516fbb0d1355641902 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sat, 5 Feb 2005 21:03:28 +0000 Subject: 2005-02-05 Havoc Pennington * dbus/dbus-marshal-validate.c (validate_body_helper): fix crash if the signature of a variant was empty (_dbus_validate_signature_with_reason): catch "(a)" (array inside struct with no element type) * dbus/dbus-message-factory.c (generate_uint32_changed): add more mangled messages to break things --- ChangeLog | 10 +++ dbus/dbus-marshal-validate-util.c | 8 +- dbus/dbus-marshal-validate.c | 26 +++++-- dbus/dbus-message-factory.c | 155 +++++++++++++++++++++++++++++++++++++- dbus/dbus-message-factory.h | 1 + 5 files changed, 190 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index dca9b596..1d35dd0c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2005-02-05 Havoc Pennington + + * dbus/dbus-marshal-validate.c (validate_body_helper): fix crash + if the signature of a variant was empty + (_dbus_validate_signature_with_reason): catch "(a)" (array inside + struct with no element type) + + * dbus/dbus-message-factory.c (generate_uint32_changed): add more + mangled messages to break things + 2005-02-04 Havoc Pennington * glib/dbus-gproxy.c (dbus_g_proxy_disconnect_signal): use diff --git a/dbus/dbus-marshal-validate-util.c b/dbus/dbus-marshal-validate-util.c index 7a1648fa..afaf5262 100644 --- a/dbus/dbus-marshal-validate-util.c +++ b/dbus/dbus-marshal-validate-util.c @@ -89,7 +89,13 @@ static const ValidityTest signature_tests[] = { { "(())", DBUS_INVALID_STRUCT_HAS_NO_FIELDS }, { "a()", DBUS_INVALID_STRUCT_HAS_NO_FIELDS }, { "i()", DBUS_INVALID_STRUCT_HAS_NO_FIELDS }, - { "()i", DBUS_INVALID_STRUCT_HAS_NO_FIELDS } + { "()i", DBUS_INVALID_STRUCT_HAS_NO_FIELDS }, + { "(a)", DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE }, + { "a{ia}", DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE }, + { "a{}", DBUS_INVALID_DICT_ENTRY_HAS_NO_FIELDS } + /* { "a{i}", DBUS_INVALID_DICT_ENTRY_HAS_ONLY_ONE_FIELD }, */ + /* { "{is}", DBUS_INVALID_DICT_ENTRY_NOT_INSIDE_ARRAY }, */ + /* { "a{isi}", DBUS_INVALID_DICT_ENTRY_HAS_TOO_MANY_FIELDS }, */ }; dbus_bool_t diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index b144b62a..f7b46c0b 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -144,8 +144,16 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, return DBUS_INVALID_UNKNOWN_TYPECODE; } - if (*p != DBUS_TYPE_ARRAY) - array_depth = 0; + if (array_depth > 0) + { + if (*p == DBUS_TYPE_ARRAY) + ; + else if (*p == DBUS_STRUCT_END_CHAR || + *p == DBUS_DICT_ENTRY_END_CHAR) + return DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE; + else + array_depth = 0; + } last = *p; ++p; @@ -159,6 +167,10 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, if (dict_entry_depth > 0) return DBUS_INVALID_DICT_ENTRY_STARTED_BUT_NOT_ENDED; + + _dbus_assert (last != DBUS_TYPE_ARRAY); + _dbus_assert (last != DBUS_STRUCT_BEGIN_CHAR); + _dbus_assert (last != DBUS_DICT_ENTRY_BEGIN_CHAR); return DBUS_VALID; } @@ -362,6 +374,7 @@ validate_body_helper (DBusTypeReader *reader, DBusTypeReader sub; DBusValidity validity; int contained_alignment; + int contained_type; claimed_len = *p; ++p; @@ -381,7 +394,11 @@ validate_body_helper (DBusTypeReader *reader, return DBUS_INVALID_VARIANT_SIGNATURE_MISSING_NUL; ++p; - contained_alignment = _dbus_type_get_alignment (_dbus_first_type_in_signature (&sig, 0)); + contained_type = _dbus_first_type_in_signature (&sig, 0); + if (contained_type == DBUS_TYPE_INVALID) + return DBUS_INVALID_VARIANT_SIGNATURE_EMPTY; + + contained_alignment = _dbus_type_get_alignment (contained_type); a = _DBUS_ALIGN_ADDRESS (p, contained_alignment); if (a > end) @@ -395,8 +412,7 @@ validate_body_helper (DBusTypeReader *reader, _dbus_type_reader_init_types_only (&sub, &sig, 0); - if (_dbus_type_reader_get_current_type (&sub) == DBUS_TYPE_INVALID) - return DBUS_INVALID_VARIANT_SIGNATURE_EMPTY; + _dbus_assert (_dbus_type_reader_get_current_type (&sub) != DBUS_TYPE_INVALID); validity = validate_body_helper (&sub, byte_order, FALSE, p, end, &p); if (validity != DBUS_VALID) diff --git a/dbus/dbus-message-factory.c b/dbus/dbus-message-factory.c index b1b90d47..26109815 100644 --- a/dbus/dbus-message-factory.c +++ b/dbus/dbus-message-factory.c @@ -28,6 +28,12 @@ #include "dbus-test.h" #include +typedef enum + { + CHANGE_TYPE_ADJUST, + CHANGE_TYPE_ABSOLUTE + } ChangeType; + #define BYTE_ORDER_OFFSET 0 #define BODY_LENGTH_OFFSET 4 @@ -36,11 +42,13 @@ iter_recurse (DBusMessageDataIter *iter) { iter->depth += 1; _dbus_assert (iter->depth < _DBUS_MESSAGE_DATA_MAX_NESTING); + _dbus_assert (iter->sequence_nos[iter->depth] >= 0); } static int iter_get_sequence (DBusMessageDataIter *iter) { + _dbus_assert (iter->sequence_nos[iter->depth] >= 0); return iter->sequence_nos[iter->depth]; } @@ -48,6 +56,7 @@ static void iter_set_sequence (DBusMessageDataIter *iter, int sequence) { + _dbus_assert (sequence >= 0); iter->sequence_nos[iter->depth] = sequence; } @@ -354,7 +363,7 @@ generate_byte_changed (DBusMessageDataIter *iter, byte_seq = iter_get_sequence (iter); iter_next (iter); iter_unrecurse (iter); - + if (byte_seq == _dbus_string_get_length (data)) { _dbus_string_set_length (data, 0); @@ -379,6 +388,137 @@ generate_byte_changed (DBusMessageDataIter *iter, return TRUE; } +typedef struct +{ + ChangeType type; + dbus_uint32_t value; /* cast to signed for adjusts */ +} UIntChange; + +static const UIntChange uint32_changes[] = { + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -1 }, + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -2 }, + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -3 }, + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 1 }, + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 2 }, + { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 3 }, + { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX }, + { CHANGE_TYPE_ABSOLUTE, 0 }, + { CHANGE_TYPE_ABSOLUTE, 1 }, + { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX - 1 }, + { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX - 5 } +}; + +static dbus_bool_t +generate_uint32_changed (DBusMessageDataIter *iter, + DBusString *data, + DBusValidity *expected_validity) +{ + int body_seq; + int byte_seq; + int change_seq; + dbus_uint32_t v_UINT32; + int byte_order; + const UIntChange *change; + int base_depth; + + /* Outer loop is each body, next loop is each change, + * inner loop is each change location + */ + + base_depth = iter->depth; + + next_body: + _dbus_assert (iter->depth == (base_depth + 0)); + _dbus_string_set_length (data, 0); + body_seq = iter_get_sequence (iter); + + if (!generate_many_bodies (iter, data, expected_validity)) + return FALSE; + + _dbus_assert (iter->depth == (base_depth + 0)); + + iter_set_sequence (iter, body_seq); /* undo the "next" from generate_many_bodies */ + iter_recurse (iter); + next_change: + _dbus_assert (iter->depth == (base_depth + 1)); + change_seq = iter_get_sequence (iter); + + if (change_seq == _DBUS_N_ELEMENTS (uint32_changes)) + { + /* Reset change count */ + iter_set_sequence (iter, 0); + iter_unrecurse (iter); + iter_next (iter); + goto next_body; + } + + _dbus_assert (iter->depth == (base_depth + 1)); + + iter_recurse (iter); + _dbus_assert (iter->depth == (base_depth + 2)); + byte_seq = iter_get_sequence (iter); + /* skip 4 bytes at a time */ + iter_next (iter); + iter_next (iter); + iter_next (iter); + iter_next (iter); + iter_unrecurse (iter); + + _dbus_assert (_DBUS_ALIGN_VALUE (byte_seq, 4) == (unsigned) byte_seq); + if (byte_seq >= (_dbus_string_get_length (data) - 4)) + { + /* reset byte count */ + _dbus_assert (iter->depth == (base_depth + 1)); + iter_recurse (iter); + _dbus_assert (iter->depth == (base_depth + 2)); + iter_set_sequence (iter, 0); + iter_unrecurse (iter); + _dbus_assert (iter->depth == (base_depth + 1)); + iter_next (iter); + goto next_change; + } + + _dbus_assert (byte_seq <= (_dbus_string_get_length (data) - 4)); + + byte_order = _dbus_string_get_byte (data, BYTE_ORDER_OFFSET); + + v_UINT32 = _dbus_marshal_read_uint32 (data, byte_seq, byte_order, NULL); + + change = &uint32_changes[change_seq]; + + if (change->type == CHANGE_TYPE_ADJUST) + { + v_UINT32 += (int) change->value; + } + else + { + v_UINT32 = change->value; + } + +#if 0 + printf ("body %d change %d pos %d ", + body_seq, change_seq, byte_seq); + + if (change->type == CHANGE_TYPE_ADJUST) + printf ("adjust by %d", (int) change->value); + else + printf ("set to %u", change->value); + + printf (" \t%u -> %u\n", + _dbus_marshal_read_uint32 (data, byte_seq, byte_order, NULL), + v_UINT32); +#endif + + _dbus_marshal_set_uint32 (data, byte_seq, v_UINT32, byte_order); + *expected_validity = DBUS_VALIDITY_UNKNOWN; + + _dbus_assert (iter->depth == (base_depth + 1)); + iter_unrecurse (iter); + _dbus_assert (iter->depth == (base_depth + 0)); + + return TRUE; +} + typedef struct { const char *name; @@ -388,6 +528,7 @@ typedef struct static const DBusMessageGenerator generators[] = { { "trivial example of each message type", generate_trivial }, { "assorted arguments", generate_many_bodies }, + { "each uint32 modified", generate_uint32_changed }, { "wrong body lengths", generate_wrong_length }, { "each byte modified", generate_byte_changed } }; @@ -409,7 +550,8 @@ _dbus_message_data_iter_init (DBusMessageDataIter *iter) { iter->sequence_nos[i] = 0; ++i; - } + } + iter->count = 0; } dbus_bool_t @@ -428,7 +570,10 @@ _dbus_message_data_iter_get_and_next (DBusMessageDataIter *iter, iter_recurse (iter); if (iter_first_in_series (iter)) - printf (" testing message loading: %s\n", generators[generator].name); + { + printf (" testing message loading: %s ", generators[generator].name); + fflush (stdout); + } func = generators[generator].func; @@ -443,10 +588,12 @@ _dbus_message_data_iter_get_and_next (DBusMessageDataIter *iter, iter_unrecurse (iter); iter_next (iter); /* next generator */ _dbus_string_free (&data->data); + printf ("%d test loads cumulative\n", iter->count); goto restart; } iter_unrecurse (iter); - + + iter->count += 1; return TRUE; } diff --git a/dbus/dbus-message-factory.h b/dbus/dbus-message-factory.h index 8d992a4f..fe52aedb 100644 --- a/dbus/dbus-message-factory.h +++ b/dbus/dbus-message-factory.h @@ -45,6 +45,7 @@ typedef struct { int sequence_nos[_DBUS_MESSAGE_DATA_MAX_NESTING]; int depth; + int count; } DBusMessageDataIter; void _dbus_message_data_free (DBusMessageData *data); -- cgit