From f7d96bdf80129d95cf33f26a778ce2c94a818bd0 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sun, 2 Mar 2003 17:34:30 +0000 Subject: 2003-03-02 Havoc Pennington * test/break-loader.c (randomly_set_extreme_ints): add test that sets really huge and small integers * dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add check that length of boolean array fits in the string, and that string has room for boolean value in single-bool case. * dbus/dbus-message-builder.c (_dbus_message_data_load): add optional value to "ALIGN" command which is what to fill the alignment with. * test/data/valid-messages/no-padding.message: add regression test for the message padding problem --- ChangeLog | 16 ++++ dbus/dbus-internals.h | 1 + dbus/dbus-marshal.c | 18 ++++- dbus/dbus-message-builder.c | 35 ++++++++- test/break-loader.c | 83 ++++++++++++++++++++- .../boolean-array-length-too-long.message-raw | Bin 0 -> 27 bytes .../boolean-has-no-value.message-raw | Bin 0 -> 102 bytes test/data/valid-messages/no-padding.message | 19 +++++ 8 files changed, 167 insertions(+), 5 deletions(-) create mode 100644 test/data/invalid-messages/boolean-array-length-too-long.message-raw create mode 100644 test/data/invalid-messages/boolean-has-no-value.message-raw create mode 100644 test/data/valid-messages/no-padding.message diff --git a/ChangeLog b/ChangeLog index 1505e69f..97b81bdf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2003-03-02 Havoc Pennington + + * test/break-loader.c (randomly_set_extreme_ints): add test that + sets really huge and small integers + + * dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add check + that length of boolean array fits in the string, and that + string has room for boolean value in single-bool case. + + * dbus/dbus-message-builder.c (_dbus_message_data_load): add + optional value to "ALIGN" command which is what to fill the + alignment with. + + * test/data/valid-messages/no-padding.message: add regression + test for the message padding problem + 2003-03-02 Havoc Pennington * dbus/dbus-message.c (decode_header_data): fix to always init diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index d928a5c8..19a5cdc3 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -115,6 +115,7 @@ char* _dbus_strdup (const char *str); #define _DBUS_INT_MIN (-_DBUS_INT_MAX - 1) #define _DBUS_INT_MAX 2147483647 +#define _DBUS_UINT_MAX 0xffffffff #define _DBUS_MAX_SUN_PATH_LENGTH 99 #define _DBUS_ONE_KILOBYTE 1024 #define _DBUS_ONE_MEGABYTE 1024 * _DBUS_ONE_KILOBYTE diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index b199561b..f78757fd 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -997,7 +997,9 @@ _dbus_marshal_get_arg_end_pos (const DBusString *str, * Demarshals and validates a length; returns < 0 if the validation * fails. The length is required to be small enough that * len*sizeof(double) will not overflow, and small enough to fit in a - * signed integer. + * signed integer. DOES NOT check whether the length points + * beyond the end of the string, because it doesn't know the + * size of array elements. * * @param str the string * @param byte_order the byte order @@ -1012,6 +1014,8 @@ demarshal_and_validate_len (const DBusString *str, { int align_4 = _DBUS_ALIGN_VALUE (pos, 4); unsigned int len; + + _dbus_assert (new_pos != NULL); if ((align_4 + 4) >= _dbus_string_get_length (str)) { @@ -1116,6 +1120,12 @@ _dbus_marshal_validate_arg (const DBusString *str, { unsigned char c; + if (2 > _dbus_string_get_length (str) - pos) + { + _dbus_verbose ("no room for boolean value\n"); + return FALSE; + } + c = _dbus_string_get_byte (str, pos + 1); if (c != 0 && c != 1) @@ -1184,6 +1194,12 @@ _dbus_marshal_validate_arg (const DBusString *str, if (len < 0) return FALSE; + if (len > _dbus_string_get_length (str) - pos) + { + _dbus_verbose ("boolean array length outside length of the message\n"); + return FALSE; + } + i = 0; while (i < len) { diff --git a/dbus/dbus-message-builder.c b/dbus/dbus-message-builder.c index dea50d7f..3501da9a 100644 --- a/dbus/dbus-message-builder.c +++ b/dbus/dbus-message-builder.c @@ -334,6 +334,12 @@ _dbus_message_data_load (DBusString *dest, _dbus_string_free (&file); return FALSE; } + + { + const char *s; + _dbus_string_get_const_data (filename, &s); + _dbus_verbose ("Loading %s\n", s); + } if ((result = _dbus_file_get_contents (&file, filename)) != DBUS_RESULT_SUCCESS) { @@ -439,24 +445,47 @@ _dbus_message_data_load (DBusString *dest, "ALIGN")) { long val; - + int end; + int orig_len; + _dbus_string_delete_first_word (&line); - if (!_dbus_string_parse_int (&line, 0, &val, NULL)) + if (!_dbus_string_parse_int (&line, 0, &val, &end)) { _dbus_warn ("Failed to parse integer\n"); goto parse_failed; } - if (val > 16) + if (val > 8) { _dbus_warn ("Aligning to %ld boundary is crack\n", val); goto parse_failed; } + + orig_len = _dbus_string_get_length (dest); if (!_dbus_string_align_length (dest, val)) goto parse_failed; + + if (_dbus_string_parse_int (&line, end, &val, NULL)) + { + /* If there's an optional second int argument, + * fill in align padding with that value + */ + if (val < 0 || val > 255) + { + _dbus_warn ("can't fill align padding with %ld, must be a byte value\n", val); + goto parse_failed; + } + + end = orig_len; + while (end < _dbus_string_get_length (dest)) + { + _dbus_string_set_byte (dest, end, val); + ++end; + } + } } else if (_dbus_string_starts_with_c_str (&line, "UNALIGN")) { diff --git a/test/break-loader.c b/test/break-loader.c index db9a14a1..f08f43d9 100644 --- a/test/break-loader.c +++ b/test/break-loader.c @@ -334,6 +334,67 @@ randomly_modify_length (const DBusString *orig_data, (unsigned) (orig + delta)); } +static void +randomly_set_extreme_ints (const DBusString *orig_data, + DBusString *mutated) +{ + int i; + int byte_order; + const char *d; + dbus_uint32_t orig; + static int which = 0; + unsigned int extreme_ints[] = { + _DBUS_INT_MAX, + _DBUS_UINT_MAX, + _DBUS_INT_MAX - 1, + _DBUS_UINT_MAX - 1, + _DBUS_INT_MAX - 2, + _DBUS_UINT_MAX - 2, + (unsigned int) (_DBUS_INT_MAX + 1), + (unsigned int) (_DBUS_UINT_MAX + 1), + _DBUS_INT_MAX + 2, + _DBUS_UINT_MAX + 2, + 0, 1, 2, 3, + (unsigned int) -1, + (unsigned int) -2, + (unsigned int) -3 + }; + + if (orig_data != mutated) + { + _dbus_string_set_length (mutated, 0); + + if (!_dbus_string_copy (orig_data, 0, mutated, 0)) + _dbus_assert_not_reached ("out of mem"); + } + + if (_dbus_string_get_length (mutated) < 12) + return; + + _dbus_string_get_const_data (mutated, &d); + + if (!(*d == DBUS_LITTLE_ENDIAN || + *d == DBUS_BIG_ENDIAN)) + return; + + byte_order = *d; + + i = random_int_in_range (4, _dbus_string_get_length (mutated) - 8); + i = _DBUS_ALIGN_VALUE (i, 4); + + orig = _dbus_demarshal_uint32 (mutated, byte_order, i, NULL); + + which = random_int_in_range (0, _DBUS_N_ELEMENTS (extreme_ints)); + + _dbus_assert (which >= 0); + _dbus_assert (which < _DBUS_N_ELEMENTS (extreme_ints)); + + _dbus_marshal_set_uint32 (mutated, byte_order, i, + extreme_ints[which]); +} + +static int times_we_did_each_thing[6] = { 0, }; + static void randomly_do_n_things (const DBusString *orig_data, DBusString *mutated, @@ -347,7 +408,8 @@ randomly_do_n_things (const DBusString *orig_data, randomly_change_one_byte, randomly_add_one_byte, randomly_remove_one_byte, - randomly_modify_length + randomly_modify_length, + randomly_set_extreme_ints }; _dbus_string_set_length (mutated, 0); @@ -363,6 +425,7 @@ randomly_do_n_things (const DBusString *orig_data, which = random_int_in_range (0, _DBUS_N_ELEMENTS (functions)); (* functions[which]) (mutated, mutated); + times_we_did_each_thing[which] += 1; ++i; } @@ -432,6 +495,15 @@ find_breaks_based_on (const DBusString *filename, ++i; } + + i = 0; + while (i < 50) + { + randomly_set_extreme_ints (&orig_data, &mutated); + try_mutated_data (&mutated); + + ++i; + } i = 0; while (i < 15) @@ -588,6 +660,15 @@ main (int argc, return 1; } + printf (" did %d random mutations: %d %d %d %d %d %d\n", + _DBUS_N_ELEMENTS (times_we_did_each_thing), + times_we_did_each_thing[0], + times_we_did_each_thing[1], + times_we_did_each_thing[2], + times_we_did_each_thing[3], + times_we_did_each_thing[4], + times_we_did_each_thing[5]); + printf ("Found %d failures with seed %u stored in %s\n", failures_this_iteration, seed, failure_dir_c); diff --git a/test/data/invalid-messages/boolean-array-length-too-long.message-raw b/test/data/invalid-messages/boolean-array-length-too-long.message-raw new file mode 100644 index 00000000..2326ec9d Binary files /dev/null and b/test/data/invalid-messages/boolean-array-length-too-long.message-raw differ diff --git a/test/data/invalid-messages/boolean-has-no-value.message-raw b/test/data/invalid-messages/boolean-has-no-value.message-raw new file mode 100644 index 00000000..cba9e839 Binary files /dev/null and b/test/data/invalid-messages/boolean-has-no-value.message-raw differ diff --git a/test/data/valid-messages/no-padding.message b/test/data/valid-messages/no-padding.message new file mode 100644 index 00000000..c21c84d3 --- /dev/null +++ b/test/data/valid-messages/no-padding.message @@ -0,0 +1,19 @@ +## Message with no header padding + +## VALID_HEADER includes a LENGTH Header and LENGTH Body +VALID_HEADER + +## this byte array is filled with zeros to the natural length +## of the header +FIELD_NAME unkn +TYPE BYTE_ARRAY +ALIGN 4 +LENGTH ThisByteArray +START_LENGTH ThisByteArray +BYTE 1 +ALIGN 8 1 +END_LENGTH ThisByteArray + +END_LENGTH Header +START_LENGTH Body +END_LENGTH Body -- cgit