diff options
| -rw-r--r-- | ChangeLog | 7 | ||||
| -rw-r--r-- | dbus/dbus-marshal-recursive-util.c | 109 | ||||
| -rw-r--r-- | dbus/dbus-marshal-recursive.c | 2 | ||||
| -rw-r--r-- | dbus/dbus-marshal-validate.c | 69 | ||||
| -rw-r--r-- | dbus/dbus-message-factory.c | 311 | ||||
| -rw-r--r-- | dbus/dbus-message-factory.h | 5 | ||||
| -rw-r--r-- | dbus/dbus-message-util.c | 11 | ||||
| -rw-r--r-- | dbus/dbus-message.c | 7 | ||||
| -rw-r--r-- | dbus/dbus-protocol.h | 8 | ||||
| -rw-r--r-- | dbus/dbus-test.h | 6 |
10 files changed, 487 insertions, 48 deletions
@@ -1,3 +1,10 @@ +2005-01-24 Havoc Pennington <hp@redhat.com> + + * dbus/dbus-message-factory.c: more testing of message validation + + * dbus/dbus-protocol.h (DBUS_MINIMUM_HEADER_SIZE): move to this + header + 2005-01-23 Havoc Pennington <hp@redhat.com> * dbus/dbus-message-factory.c, dbus/dbus-message-util.c: diff --git a/dbus/dbus-marshal-recursive-util.c b/dbus/dbus-marshal-recursive-util.c index e257a51c..493672b6 100644 --- a/dbus/dbus-marshal-recursive-util.c +++ b/dbus/dbus-marshal-recursive-util.c @@ -1425,6 +1425,87 @@ value_generator (int *ip) } static void +build_body (TestTypeNode **nodes, + int n_nodes, + int byte_order, + DBusString *signature, + DBusString *body) +{ + int i; + DataBlock block; + DBusTypeReader reader; + DBusTypeWriter writer; + + i = 0; + while (i < n_nodes) + { + if (! node_build_signature (nodes[i], signature)) + _dbus_assert_not_reached ("no memory"); + + ++i; + } + + if (!data_block_init (&block, byte_order, 0)) + _dbus_assert_not_reached ("no memory"); + + data_block_init_reader_writer (&block, + &reader, &writer); + + /* DBusTypeWriter assumes it's writing into an existing signature, + * so doesn't add nul on its own. We have to do that. + */ + if (!_dbus_string_insert_byte (&block.signature, + 0, '\0')) + _dbus_assert_not_reached ("no memory"); + + i = 0; + while (i < n_nodes) + { + if (!node_write_value (nodes[i], &block, &writer, i)) + _dbus_assert_not_reached ("no memory"); + + ++i; + } + + if (!_dbus_string_copy_len (&block.body, 0, + _dbus_string_get_length (&block.body) - N_FENCE_BYTES, + body, 0)) + _dbus_assert_not_reached ("oom"); + + data_block_free (&block); +} + +dbus_bool_t +dbus_internal_do_not_use_generate_bodies (int sequence, + int byte_order, + DBusString *signature, + DBusString *body) +{ + TestTypeNode *nodes[1]; + int i; + int n_nodes; + + nodes[0] = value_generator (&sequence); + + if (nodes[0] == NULL) + return FALSE; + + n_nodes = 1; + + build_body (nodes, n_nodes, byte_order, signature, body); + + + i = 0; + while (i < n_nodes) + { + node_destroy (nodes[i]); + ++i; + } + + return TRUE; +} + +static void make_and_run_values_inside_container (const TestTypeNodeClass *container_klass, int n_nested) { @@ -2305,18 +2386,26 @@ object_path_from_seed (char *buf, v = (unsigned char) ('A' + seed); - i = 0; - while (i + 1 < len) + if (len < 2) { - if (v < 'A' || v > 'z') - v = 'A'; - - buf[i] = '/'; - ++i; - buf[i] = v; - ++i; + buf[0] = '/'; + i = 1; + } + else + { + i = 0; + while (i + 1 < len) + { + if (v < 'A' || v > 'z') + v = 'A'; - v += 1; + buf[i] = '/'; + ++i; + buf[i] = v; + ++i; + + v += 1; + } } buf[i] = '\0'; diff --git a/dbus/dbus-marshal-recursive.c b/dbus/dbus-marshal-recursive.c index e98d1c35..239a1fcb 100644 --- a/dbus/dbus-marshal-recursive.c +++ b/dbus/dbus-marshal-recursive.c @@ -923,7 +923,7 @@ _dbus_type_reader_read_fixed_multi (const DBusTypeReader *reader, _dbus_assert (reader->klass == &array_reader_class); element_type = _dbus_first_type_in_signature (reader->type_str, - reader->type_pos); + reader->type_pos); _dbus_assert (element_type != DBUS_TYPE_INVALID); /* why we don't use get_current_type() */ _dbus_assert (_dbus_type_is_fixed (element_type)); diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index ad61847e..f2e0c39d 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -193,7 +193,7 @@ validate_body_helper (DBusTypeReader *reader, dbus_uint32_t claimed_len; a = _DBUS_ALIGN_ADDRESS (p, 4); - if (a + 4 >= end) + if (a + 4 > end) return DBUS_INVALID_NOT_ENOUGH_DATA; while (p != a) { @@ -205,6 +205,9 @@ validate_body_helper (DBusTypeReader *reader, claimed_len = _dbus_unpack_uint32 (byte_order, p); p += 4; + /* p may now be == end */ + _dbus_assert (p <= end); + if (current_type == DBUS_TYPE_ARRAY) { int array_elem_type = _dbus_type_reader_get_element_type (reader); @@ -1322,6 +1325,70 @@ _dbus_marshal_validate_test (void) _dbus_string_free (&str); + /* Body validation; test basic validation of valid bodies for both endian */ + + { + int sequence; + DBusString signature; + DBusString body; + + if (!_dbus_string_init (&signature) || !_dbus_string_init (&body)) + _dbus_assert_not_reached ("oom"); + + sequence = 0; + while (dbus_internal_do_not_use_generate_bodies (sequence, + DBUS_LITTLE_ENDIAN, + &signature, &body)) + { + DBusValidity validity; + + validity = _dbus_validate_body_with_reason (&signature, 0, + DBUS_LITTLE_ENDIAN, + NULL, &body, 0, + _dbus_string_get_length (&body)); + if (validity != DBUS_VALID) + { + _dbus_warn ("invalid code %d expected valid on sequence %d little endian\n", + validity, sequence); + _dbus_verbose_bytes_of_string (&signature, 0, _dbus_string_get_length (&signature)); + _dbus_verbose_bytes_of_string (&body, 0, _dbus_string_get_length (&body)); + _dbus_assert_not_reached ("test failed"); + } + + _dbus_string_set_length (&signature, 0); + _dbus_string_set_length (&body, 0); + ++sequence; + } + + sequence = 0; + while (dbus_internal_do_not_use_generate_bodies (sequence, + DBUS_BIG_ENDIAN, + &signature, &body)) + { + DBusValidity validity; + + validity = _dbus_validate_body_with_reason (&signature, 0, + DBUS_BIG_ENDIAN, + NULL, &body, 0, + _dbus_string_get_length (&body)); + if (validity != DBUS_VALID) + { + _dbus_warn ("invalid code %d expected valid on sequence %d big endian\n", + validity, sequence); + _dbus_verbose_bytes_of_string (&signature, 0, _dbus_string_get_length (&signature)); + _dbus_verbose_bytes_of_string (&body, 0, _dbus_string_get_length (&body)); + _dbus_assert_not_reached ("test failed"); + } + + _dbus_string_set_length (&signature, 0); + _dbus_string_set_length (&body, 0); + ++sequence; + } + + _dbus_string_free (&signature); + _dbus_string_free (&body); + } + return TRUE; } diff --git a/dbus/dbus-message-factory.c b/dbus/dbus-message-factory.c index d4b89565..b1b90d47 100644 --- a/dbus/dbus-message-factory.c +++ b/dbus/dbus-message-factory.c @@ -25,12 +25,65 @@ #ifdef DBUS_BUILD_TESTS #include "dbus-message-factory.h" #include "dbus-message-private.h" +#include "dbus-test.h" +#include <stdio.h> -typedef dbus_bool_t (* DBusInnerGeneratorFunc) (int sequence, - DBusMessage **message_p); -typedef dbus_bool_t (* DBusMessageGeneratorFunc) (int sequence, - DBusString *data, - DBusValidity *expected_validity); +#define BYTE_ORDER_OFFSET 0 +#define BODY_LENGTH_OFFSET 4 + +static void +iter_recurse (DBusMessageDataIter *iter) +{ + iter->depth += 1; + _dbus_assert (iter->depth < _DBUS_MESSAGE_DATA_MAX_NESTING); +} + +static int +iter_get_sequence (DBusMessageDataIter *iter) +{ + return iter->sequence_nos[iter->depth]; +} + +static void +iter_set_sequence (DBusMessageDataIter *iter, + int sequence) +{ + iter->sequence_nos[iter->depth] = sequence; +} + +static void +iter_unrecurse (DBusMessageDataIter *iter) +{ + iter->depth -= 1; + _dbus_assert (iter->depth >= 0); +} + +static void +iter_next (DBusMessageDataIter *iter) +{ + iter->sequence_nos[iter->depth] += 1; +} + +static dbus_bool_t +iter_first_in_series (DBusMessageDataIter *iter) +{ + int i; + + i = iter->depth; + while (i < _DBUS_MESSAGE_DATA_MAX_NESTING) + { + if (iter->sequence_nos[i] != 0) + return FALSE; + ++i; + } + return TRUE; +} + +typedef dbus_bool_t (* DBusInnerGeneratorFunc) (DBusMessageDataIter *iter, + DBusMessage **message_p); +typedef dbus_bool_t (* DBusMessageGeneratorFunc) (DBusMessageDataIter *iter, + DBusString *data, + DBusValidity *expected_validity); static void set_reply_serial (DBusMessage *message) @@ -42,12 +95,12 @@ set_reply_serial (DBusMessage *message) } static dbus_bool_t -generate_trivial_inner (int sequence, - DBusMessage **message_p) +generate_trivial_inner (DBusMessageDataIter *iter, + DBusMessage **message_p) { DBusMessage *message; - switch (sequence) + switch (iter_get_sequence (iter)) { case 0: message = dbus_message_new_method_call ("org.freedesktop.TextEditor", @@ -97,7 +150,61 @@ generate_trivial_inner (int sequence, } static dbus_bool_t -generate_outer (int sequence, +generate_many_bodies_inner (DBusMessageDataIter *iter, + DBusMessage **message_p) +{ + DBusMessage *message; + DBusString signature; + DBusString body; + + message = dbus_message_new_method_call ("org.freedesktop.Foo", + "/", + "org.freedesktop.Blah", + "NahNahNah"); + if (message == NULL) + _dbus_assert_not_reached ("oom"); + + set_reply_serial (message); + + if (!_dbus_string_init (&signature) || !_dbus_string_init (&body)) + _dbus_assert_not_reached ("oom"); + + if (dbus_internal_do_not_use_generate_bodies (iter_get_sequence (iter), + message->byte_order, + &signature, &body)) + { + const char *v_SIGNATURE; + + v_SIGNATURE = _dbus_string_get_const_data (&signature); + if (!_dbus_header_set_field_basic (&message->header, + DBUS_HEADER_FIELD_SIGNATURE, + DBUS_TYPE_SIGNATURE, + &v_SIGNATURE)) + _dbus_assert_not_reached ("oom"); + + if (!_dbus_string_move (&body, 0, &message->body, 0)) + _dbus_assert_not_reached ("oom"); + + _dbus_marshal_set_uint32 (&message->header.data, BODY_LENGTH_OFFSET, + _dbus_string_get_length (&message->body), + message->byte_order); + + *message_p = message; + } + else + { + dbus_message_unref (message); + *message_p = NULL; + } + + _dbus_string_free (&signature); + _dbus_string_free (&body); + + return *message_p != NULL; +} + +static dbus_bool_t +generate_outer (DBusMessageDataIter *iter, DBusString *data, DBusValidity *expected_validity, DBusInnerGeneratorFunc func) @@ -105,9 +212,11 @@ generate_outer (int sequence, DBusMessage *message; message = NULL; - if (!(*func)(sequence, &message)) + if (!(*func)(iter, &message)) return FALSE; + iter_next (iter); + _dbus_assert (message != NULL); _dbus_message_set_serial (message, 1); @@ -130,16 +239,157 @@ generate_outer (int sequence, } static dbus_bool_t -generate_trivial (int sequence, +generate_trivial (DBusMessageDataIter *iter, DBusString *data, DBusValidity *expected_validity) { - return generate_outer (sequence, data, expected_validity, + return generate_outer (iter, data, expected_validity, generate_trivial_inner); } -static const DBusMessageGeneratorFunc generators[] = { - generate_trivial +static dbus_bool_t +generate_many_bodies (DBusMessageDataIter *iter, + DBusString *data, + DBusValidity *expected_validity) +{ + return generate_outer (iter, data, expected_validity, + generate_many_bodies_inner); +} + +static dbus_bool_t +generate_wrong_length (DBusMessageDataIter *iter, + DBusString *data, + DBusValidity *expected_validity) +{ + int lengths[] = { -42, -17, -16, -15, -9, -8, -7, -6, -5, -4, -3, -2, -1, + 1, 2, 3, 4, 5, 6, 7, 8, 9, 15, 16, 30 }; + int adjust; + int len_seq; + + restart: + len_seq = iter_get_sequence (iter); + if (len_seq == _DBUS_N_ELEMENTS (lengths)) + return FALSE; + + _dbus_assert (len_seq < _DBUS_N_ELEMENTS (lengths)); + + iter_recurse (iter); + if (!generate_many_bodies (iter, data, expected_validity)) + { + iter_set_sequence (iter, 0); /* reset to first body */ + iter_unrecurse (iter); + iter_next (iter); /* next length adjustment */ + goto restart; + } + iter_unrecurse (iter); + + adjust = lengths[len_seq]; + + if (adjust < 0) + { + if ((_dbus_string_get_length (data) + adjust) < DBUS_MINIMUM_HEADER_SIZE) + _dbus_string_set_length (data, DBUS_MINIMUM_HEADER_SIZE); + else + _dbus_string_shorten (data, - adjust); + *expected_validity = DBUS_INVALID_FOR_UNKNOWN_REASON; + } + else + { + if (!_dbus_string_lengthen (data, adjust)) + _dbus_assert_not_reached ("oom"); + *expected_validity = DBUS_INVALID_TOO_MUCH_DATA; + } + + /* Fixup lengths */ + { + int old_body_len; + int new_body_len; + int byte_order; + + _dbus_assert (_dbus_string_get_length (data) >= DBUS_MINIMUM_HEADER_SIZE); + + byte_order = _dbus_string_get_byte (data, BYTE_ORDER_OFFSET); + old_body_len = _dbus_marshal_read_uint32 (data, + BODY_LENGTH_OFFSET, + byte_order, + NULL); + _dbus_assert (old_body_len < _dbus_string_get_length (data)); + new_body_len = old_body_len + adjust; + if (new_body_len < 0) + { + new_body_len = 0; + /* we just munged the header, and aren't sure how */ + *expected_validity = DBUS_VALIDITY_UNKNOWN; + } + + _dbus_verbose ("changing body len from %u to %u by adjust %d\n", + old_body_len, new_body_len, adjust); + + _dbus_marshal_set_uint32 (data, BODY_LENGTH_OFFSET, + new_body_len, + byte_order); + } + + return TRUE; +} + +static dbus_bool_t +generate_byte_changed (DBusMessageDataIter *iter, + DBusString *data, + DBusValidity *expected_validity) +{ + int byte_seq; + int v_BYTE; + + /* This is a little convoluted to make the bodies the + * outer loop and each byte of each body the inner + * loop + */ + + restart: + if (!generate_many_bodies (iter, data, expected_validity)) + return FALSE; + + iter_recurse (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); + /* reset byte count */ + iter_recurse (iter); + iter_set_sequence (iter, 0); + iter_unrecurse (iter); + goto restart; + } + else + { + /* Undo the "next" in generate_many_bodies */ + iter_set_sequence (iter, iter_get_sequence (iter) - 1); + } + + _dbus_assert (byte_seq < _dbus_string_get_length (data)); + v_BYTE = _dbus_string_get_byte (data, byte_seq); + v_BYTE += byte_seq; /* arbitrary but deterministic change to the byte */ + _dbus_string_set_byte (data, byte_seq, v_BYTE); + *expected_validity = DBUS_VALIDITY_UNKNOWN; + + return TRUE; +} + +typedef struct +{ + const char *name; + DBusMessageGeneratorFunc func; +} DBusMessageGenerator; + +static const DBusMessageGenerator generators[] = { + { "trivial example of each message type", generate_trivial }, + { "assorted arguments", generate_many_bodies }, + { "wrong body lengths", generate_wrong_length }, + { "each byte modified", generate_byte_changed } }; void @@ -151,8 +401,15 @@ _dbus_message_data_free (DBusMessageData *data) void _dbus_message_data_iter_init (DBusMessageDataIter *iter) { - iter->generator = 0; - iter->sequence = 0; + int i; + + iter->depth = 0; + i = 0; + while (i < _DBUS_MESSAGE_DATA_MAX_NESTING) + { + iter->sequence_nos[i] = 0; + ++i; + } } dbus_bool_t @@ -160,25 +417,35 @@ _dbus_message_data_iter_get_and_next (DBusMessageDataIter *iter, DBusMessageData *data) { DBusMessageGeneratorFunc func; + int generator; restart: - if (iter->generator == _DBUS_N_ELEMENTS (generators)) + generator = iter_get_sequence (iter); + + if (generator == _DBUS_N_ELEMENTS (generators)) return FALSE; + + iter_recurse (iter); + + if (iter_first_in_series (iter)) + printf (" testing message loading: %s\n", generators[generator].name); - func = generators[iter->generator]; + func = generators[generator].func; if (!_dbus_string_init (&data->data)) _dbus_assert_not_reached ("oom"); - if ((*func)(iter->sequence, &data->data, &data->expected_validity)) - iter->sequence += 1; + if ((*func)(iter, &data->data, &data->expected_validity)) + ; else { - iter->generator += 1; - iter->sequence = 0; + iter_set_sequence (iter, 0); + iter_unrecurse (iter); + iter_next (iter); /* next generator */ _dbus_string_free (&data->data); goto restart; } + iter_unrecurse (iter); return TRUE; } diff --git a/dbus/dbus-message-factory.h b/dbus/dbus-message-factory.h index fb97ab84..8d992a4f 100644 --- a/dbus/dbus-message-factory.h +++ b/dbus/dbus-message-factory.h @@ -40,10 +40,11 @@ typedef struct } DBusMessageData; +#define _DBUS_MESSAGE_DATA_MAX_NESTING 10 typedef struct { - int generator; - int sequence; + int sequence_nos[_DBUS_MESSAGE_DATA_MAX_NESTING]; + int depth; } DBusMessageDataIter; void _dbus_message_data_free (DBusMessageData *data); diff --git a/dbus/dbus-message-util.c b/dbus/dbus-message-util.c index 5d503a98..2d72f71e 100644 --- a/dbus/dbus-message-util.c +++ b/dbus/dbus-message-util.c @@ -1148,7 +1148,9 @@ _dbus_message_test (const char *test_data_dir) { DBusMessageDataIter diter; DBusMessageData mdata; + int count; + count = 0; _dbus_message_data_iter_init (&diter); while (_dbus_message_data_iter_get_and_next (&diter, @@ -1157,14 +1159,17 @@ _dbus_message_test (const char *test_data_dir) if (!dbus_internal_do_not_use_try_message_data (&mdata.data, mdata.expected_validity)) { - _dbus_warn ("expected validity %d and did not get it; generator %d sequence %d\n", - mdata.expected_validity, - diter.generator, diter.sequence); + _dbus_warn ("expected validity %d and did not get it\n", + mdata.expected_validity); _dbus_assert_not_reached ("message data failed"); } _dbus_message_data_free (&mdata); + + count += 1; } + + printf ("%d sample messages tested\n", count); } check_memleaks (); diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index eca2a3c4..750d2344 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2970,13 +2970,6 @@ _dbus_message_loader_get_buffer (DBusMessageLoader *loader, } /** - * The smallest header size that can occur. (It won't be valid due to - * missing required header fields.) This is 4 bytes, two uint32, an - * array length. - */ -#define DBUS_MINIMUM_HEADER_SIZE 16 - -/** * Returns a buffer obtained from _dbus_message_loader_get_buffer(), * indicating to the loader how many bytes of the buffer were filled * in. This function must always be called, even if no bytes were diff --git a/dbus/dbus-protocol.h b/dbus/dbus-protocol.h index 385c5ebc..9f569ec9 100644 --- a/dbus/dbus-protocol.h +++ b/dbus/dbus-protocol.h @@ -184,6 +184,14 @@ extern "C" { DBUS_STRUCT_END_CHAR_AS_STRING +/** + * The smallest header size that can occur. (It won't be valid due to + * missing required header fields.) This is 4 bytes, two uint32, an + * array length. This isn't any kind of resource limit, just the + * necessary/logical outcome of the header signature. + */ +#define DBUS_MINIMUM_HEADER_SIZE 16 + /* Services */ #define DBUS_SERVICE_ORG_FREEDESKTOP_DBUS "org.freedesktop.DBus" diff --git a/dbus/dbus-test.h b/dbus/dbus-test.h index 06d368fa..9a63914f 100644 --- a/dbus/dbus-test.h +++ b/dbus/dbus-test.h @@ -69,8 +69,10 @@ typedef dbus_bool_t (* DBusForeachMessageFileFunc) (const DBusString *filename dbus_bool_t dbus_internal_do_not_use_foreach_message_file (const char *test_data_dir, DBusForeachMessageFileFunc func, void *user_data); - - +dbus_bool_t dbus_internal_do_not_use_generate_bodies (int sequence, + int byte_order, + DBusString *signature, + DBusString *body); #endif /* DBUS_TEST_H */ |