diff options
| -rw-r--r-- | ChangeLog | 46 | ||||
| -rw-r--r-- | dbus/Makefile.am | 5 | ||||
| -rw-r--r-- | dbus/dbus-internals.c | 2 | ||||
| -rw-r--r-- | dbus/dbus-marshal.c | 113 | ||||
| -rw-r--r-- | dbus/dbus-marshal.h | 41 | ||||
| -rw-r--r-- | dbus/dbus-message.c | 194 | ||||
| -rw-r--r-- | dbus/dbus-message.h | 4 | ||||
| -rw-r--r-- | dbus/dbus-string.c | 35 | ||||
| -rw-r--r-- | dbus/dbus-string.h | 14 | ||||
| -rw-r--r-- | dbus/dbus-test-main.c | 36 | ||||
| -rw-r--r-- | dbus/dbus-test.c | 30 | ||||
| -rw-r--r-- | dbus/dbus-test.h | 2 |
12 files changed, 400 insertions, 122 deletions
@@ -1,3 +1,49 @@ +2003-01-08 Havoc Pennington <hp@pobox.com> + + * dbus/dbus-string.c (_dbus_string_align_length): new function + + * dbus/dbus-test-main.c: move main() for test app here + * dbus/dbus-test.c + (dbus_internal_symbol_do_not_use_run_tests): we have to export a + symbol to run tests, because dbus-test isn't in the main + library + + Code review nitpicks. + + * dbus/dbus-message.c (dbus_message_write_header): add newlines + for people with narrow emacs ;-). Assert client_serial was filled + in. Assert message->name != NULL. + (dbus_message_append_fields): have "first_field_type" arg separate + from va list, needed for C++ binding that also uses varargs IIRC + and helps with type safety + (dbus_message_new): add @todo about using DBusString to store + service/name internally + (dbus_message_new): don't leak ->service and ->name on OOM later + in the function + (dbus_message_unref): free the service name + (dbus_message_get_fields): same change to varargs + i.e. first_field_type + (_dbus_message_loader_return_buffer): assert that the message data + is aligned (if not it's a bug in our code). Put in verbose griping + about why we set corrupted = TRUE. + (decode_header_data): add FIXME that char* is evil. Was going to + add FIXME about evil locale-specific string.h strncmp, but just + switched to wacky string-as-uint32 optimization. Move check for + "no room for field name" above get_const_data_len() to avoid + assertion failure in get_const_data_len if we have trailing 2 + bytes or the like. Check for service and name fields being + provided twice. Don't leak service/name on error. Require field + names to be aligned to 4 bytes. + + * dbus/dbus-marshal.c: move byte swap stuff to header + (_dbus_pack_int32): uscore-prefix + (_dbus_unpack_int32): uscore-prefix + (_dbus_unpack_uint32): export + (_dbus_demarshal_string): add @todo complaining about use of + memcpy() + (_dbus_marshal_get_field_end_pos): add @todo about bad error + handling allowing corrupt data to go unchecked + 2003-01-08 Havoc Pennington <hp@redhat.com> * dbus/dbus-transport-unix.c (unix_do_iteration): add read/write diff --git a/dbus/Makefile.am b/dbus/Makefile.am index dfc1a561..3457534d 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -32,6 +32,8 @@ libdbus_1_la_SOURCES= \ dbus-server-protected.h \ dbus-server-unix.c \ dbus-server-unix.h \ + dbus-test.c \ + dbus-test.h \ dbus-threads.c \ dbus-transport.c \ dbus-transport.h \ @@ -75,8 +77,7 @@ if DBUS_BUILD_TESTS noinst_PROGRAMS=dbus-test dbus_test_SOURCES= \ - dbus-test.c \ - dbus-test.h + dbus-test-main.c dbus_test_LDADD= $(DBUS_CLIENT_LIBS) libdbus-convenience.la libdbus-1.la diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index add963e9..9b672ef6 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -22,6 +22,7 @@ */ #include "dbus-internals.h" #include "dbus-protocol.h" +#include "dbus-test.h" #include <stdio.h> #include <stdarg.h> #include <string.h> @@ -29,6 +30,7 @@ #include <errno.h> #include <unistd.h> #include <fcntl.h> +#include <stdlib.h> /** * @defgroup DBusInternals D-BUS internal implementation details diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c index 14629362..a6055806 100644 --- a/dbus/dbus-marshal.c +++ b/dbus/dbus-marshal.c @@ -26,33 +26,6 @@ #include <string.h> -#define DBUS_UINT32_SWAP_LE_BE_CONSTANT(val) ((dbus_uint32_t) ( \ - (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x000000ffU) << 24) | \ - (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x0000ff00U) << 8) | \ - (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x00ff0000U) >> 8) | \ - (((dbus_uint32_t) (val) & (dbus_uint32_t) 0xff000000U) >> 24))) - -#define DBUS_UINT32_SWAP_LE_BE(val) (DBUS_UINT32_SWAP_LE_BE_CONSTANT (val)) - -#ifdef WORDS_BIGENDIAN -#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) (val)) -#define DBUS_UINT32_TO_BE(val) ((dbus_uint32_t) (val)) -#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) -#define DBUS_UINT32_TO_LE(val) (DBUS_UINT32_SWAP_LE_BE (val)) -#else -#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) (val)) -#define DBUS_UINT32_TO_LE(val) ((dbus_uint32_t) (val)) -#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) -#define DBUS_UINT32_TO_BE(val) (DBUS_UINT32_SWAP_LE_BE (val)) -#endif - -/* The transformation is symmetric, so the FROM just maps to the TO. */ -#define DBUS_INT32_FROM_LE(val) (DBUS_INT32_TO_LE (val)) -#define DBUS_UINT32_FROM_LE(val) (DBUS_UINT32_TO_LE (val)) -#define DBUS_INT32_FROM_BE(val) (DBUS_INT32_TO_BE (val)) -#define DBUS_UINT32_FROM_BE(val) (DBUS_UINT32_TO_BE (val)) - - /* from ORBit */ static void swap_bytes (unsigned char *data, @@ -72,9 +45,27 @@ swap_bytes (unsigned char *data, } } -static dbus_uint32_t -unpack_uint32 (int byte_order, - const unsigned char *data) +/** + * @defgroup DBusMarshal marshaling and unmarshaling + * @ingroup DBusInternals + * @brief functions to marshal/unmarshal data from the wire + * + * Types and functions related to converting primitive data types from + * wire format to native machine format, and vice versa. + * + * @{ + */ + +/** + * Unpacks a 32 bit unsigned integer from a data pointer + * + * @param byte_order The byte order to use + * @param data the data pointer + * @returns the integer + */ +dbus_uint32_t +_dbus_unpack_uint32 (int byte_order, + const unsigned char *data) { _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data); @@ -85,15 +76,15 @@ unpack_uint32 (int byte_order, } /** - * Unpacks a 32 bit unsigned integer from a data pointer + * Unpacks a 32 bit signed integer from a data pointer * * @param byte_order The byte order to use * @param data the data pointer * @returns the integer */ dbus_int32_t -dbus_unpack_int32 (int byte_order, - const unsigned char *data) +_dbus_unpack_int32 (int byte_order, + const unsigned char *data) { _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data); @@ -104,27 +95,36 @@ dbus_unpack_int32 (int byte_order, } /** - * @defgroup DBusMarshal marshaling and unmarshaling - * @ingroup DBusInternals - * @brief functions to marshal/unmarshal data from the wire - * - * Types and functions related to converting primitive data types from - * wire format to native machine format, and vice versa. + * Packs a 32 bit unsigned integer into a data pointer. * - * @{ + * @param value the value + * @param byte_order the byte order to use + * @param data the data pointer */ +void +_dbus_pack_uint32 (dbus_uint32_t value, + int byte_order, + unsigned char *data) +{ + _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data); + + if ((byte_order) == DBUS_LITTLE_ENDIAN) + *((dbus_uint32_t*)(data)) = DBUS_UINT32_TO_LE (value); + else + *((dbus_uint32_t*)(data)) = DBUS_UINT32_TO_BE (value); +} /** - * Packs a 32 bit unsigned integer into a data pointer. + * Packs a 32 bit signed integer into a data pointer. * * @param value the value * @param byte_order the byte order to use * @param data the data pointer */ void -dbus_pack_int32 (dbus_int32_t value, - int byte_order, - unsigned char *data) +_dbus_pack_int32 (dbus_int32_t value, + int byte_order, + unsigned char *data) { _dbus_assert (_DBUS_ALIGN_ADDRESS (data, 4) == data); @@ -147,9 +147,11 @@ _dbus_marshal_double (DBusString *str, int byte_order, double value) { + _dbus_assert (sizeof (double) == 8); + if (!_dbus_string_set_length (str, _DBUS_ALIGN_VALUE (_dbus_string_get_length (str), - sizeof (double)))) + sizeof (double)))) return FALSE; if (byte_order != DBUS_COMPILER_BYTE_ORDER) @@ -173,7 +175,7 @@ _dbus_marshal_int32 (DBusString *str, { if (!_dbus_string_set_length (str, _DBUS_ALIGN_VALUE (_dbus_string_get_length (str), - sizeof (dbus_int32_t)))) + sizeof (dbus_int32_t)))) return FALSE; if (byte_order != DBUS_COMPILER_BYTE_ORDER) @@ -197,7 +199,7 @@ _dbus_marshal_uint32 (DBusString *str, { if (!_dbus_string_set_length (str, _DBUS_ALIGN_VALUE (_dbus_string_get_length (str), - sizeof (dbus_uint32_t)))) + sizeof (dbus_uint32_t)))) return FALSE; if (byte_order != DBUS_COMPILER_BYTE_ORDER) @@ -323,7 +325,7 @@ _dbus_demarshal_int32 (DBusString *str, if (new_pos) *new_pos = pos + sizeof (dbus_int32_t); - return dbus_unpack_int32 (byte_order, buffer); + return _dbus_unpack_int32 (byte_order, buffer); } /** @@ -350,7 +352,7 @@ _dbus_demarshal_uint32 (DBusString *str, if (new_pos) *new_pos = pos + sizeof (dbus_uint32_t); - return unpack_uint32 (byte_order, buffer); + return _dbus_unpack_uint32 (byte_order, buffer); } /** @@ -360,6 +362,9 @@ _dbus_demarshal_uint32 (DBusString *str, * that it's valid UTF-8, and maybe "fix" the string * if it's broken? * + * @todo Should probably demarshal to a DBusString, + * having memcpy() in here is Evil(tm). + * * @param str the string containing the data * @param byte_order the byte order * @param pos the position in the string @@ -434,6 +439,12 @@ _dbus_demarshal_byte_array (DBusString *str, * Returns the position right after the end position * end position of a field * + * @todo warns on invalid type in a message, but + * probably the whole message needs to be dumped, + * or we might even drop the connection due + * to bad protocol. Needs better error handling. + * Possible security issue. + * * @param str a string * @param byte_order the byte order to use * @param pos the pos where the field starts @@ -564,8 +575,8 @@ _dbus_verbose_bytes (const unsigned char *data, { if (i > 3) _dbus_verbose ("big: %d little: %d", - unpack_uint32 (DBUS_BIG_ENDIAN, &data[i-4]), - unpack_uint32 (DBUS_LITTLE_ENDIAN, &data[i-4])); + _dbus_unpack_uint32 (DBUS_BIG_ENDIAN, &data[i-4]), + _dbus_unpack_uint32 (DBUS_LITTLE_ENDIAN, &data[i-4])); _dbus_verbose ("\n"); } diff --git a/dbus/dbus-marshal.h b/dbus/dbus-marshal.h index 4f1ba37e..1c4c0e40 100644 --- a/dbus/dbus-marshal.h +++ b/dbus/dbus-marshal.h @@ -39,11 +39,42 @@ #define DBUS_COMPILER_BYTE_ORDER DBUS_LITTLE_ENDIAN #endif -void dbus_pack_int32 (dbus_int32_t value, - int byte_order, - unsigned char *data); -dbus_int32_t dbus_unpack_int32 (int byte_order, - const unsigned char *data); +#define DBUS_UINT32_SWAP_LE_BE_CONSTANT(val) ((dbus_uint32_t) ( \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x000000ffU) << 24) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x0000ff00U) << 8) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0x00ff0000U) >> 8) | \ + (((dbus_uint32_t) (val) & (dbus_uint32_t) 0xff000000U) >> 24))) + +#define DBUS_UINT32_SWAP_LE_BE(val) (DBUS_UINT32_SWAP_LE_BE_CONSTANT (val)) + +#ifdef WORDS_BIGENDIAN +#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) (val)) +#define DBUS_UINT32_TO_BE(val) ((dbus_uint32_t) (val)) +#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) +#define DBUS_UINT32_TO_LE(val) (DBUS_UINT32_SWAP_LE_BE (val)) +#else +#define DBUS_INT32_TO_LE(val) ((dbus_int32_t) (val)) +#define DBUS_UINT32_TO_LE(val) ((dbus_uint32_t) (val)) +#define DBUS_INT32_TO_BE(val) ((dbus_int32_t) DBUS_UINT32_SWAP_LE_BE (val)) +#define DBUS_UINT32_TO_BE(val) (DBUS_UINT32_SWAP_LE_BE (val)) +#endif + +/* The transformation is symmetric, so the FROM just maps to the TO. */ +#define DBUS_INT32_FROM_LE(val) (DBUS_INT32_TO_LE (val)) +#define DBUS_UINT32_FROM_LE(val) (DBUS_UINT32_TO_LE (val)) +#define DBUS_INT32_FROM_BE(val) (DBUS_INT32_TO_BE (val)) +#define DBUS_UINT32_FROM_BE(val) (DBUS_UINT32_TO_BE (val)) + +void _dbus_pack_int32 (dbus_int32_t value, + int byte_order, + unsigned char *data); +dbus_int32_t _dbus_unpack_int32 (int byte_order, + const unsigned char *data); +void _dbus_pack_uint32 (dbus_uint32_t value, + int byte_order, + unsigned char *data); +dbus_uint32_t _dbus_unpack_uint32 (int byte_order, + const unsigned char *data); dbus_bool_t _dbus_marshal_double (DBusString *str, diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 49269247..041bf667 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -76,6 +76,11 @@ struct DBusMessage unsigned int locked : 1; /**< Message being sent, no modifications allowed. */ }; +/** + * @brief Internals of DBusMessageIter + * + * Object representing a position in a message. All fields are internal. + */ struct DBusMessageIter { int refcount; /**< Reference count */ @@ -125,7 +130,7 @@ _dbus_message_set_client_serial (DBusMessage *message, static void dbus_message_write_header (DBusMessage *message) { - char *header; + char *len_data; _dbus_assert (message->client_serial != -1); @@ -135,21 +140,28 @@ dbus_message_write_header (DBusMessage *message) /* We just lengthen the string here and pack in the real length later */ _dbus_string_lengthen (&message->header, 4); - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, _dbus_string_get_length (&message->body)); + _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, + _dbus_string_get_length (&message->body)); /* Marshal client serial */ - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, message->client_serial); + _dbus_assert (message->client_serial >= 0); + _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, + message->client_serial); /* Marshal message service */ if (message->service) { + _dbus_string_align_length (&message->header, 4); _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_SERVICE, 4); _dbus_string_append_byte (&message->header, DBUS_TYPE_STRING); - _dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, message->service); + _dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER, + message->service); } /* Marshal message name */ + _dbus_assert (message->name != NULL); + _dbus_string_align_length (&message->header, 4); _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_NAME, 4); _dbus_string_append_byte (&message->header, DBUS_TYPE_STRING); @@ -161,12 +173,14 @@ dbus_message_write_header (DBusMessage *message) _dbus_string_append_len (&message->header, DBUS_HEADER_FIELD_REPLY, 4); _dbus_string_append_byte (&message->header, DBUS_TYPE_INT32); - _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, message->reply_serial); + _dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER, + message->reply_serial); } /* Fill in the length */ - _dbus_string_get_data_len (&message->header, &header, 4, 4); - dbus_pack_int32 (_dbus_string_get_length (&message->header), DBUS_COMPILER_BYTE_ORDER, header); + _dbus_string_get_data_len (&message->header, &len_data, 4, 4); + _dbus_pack_int32 (_dbus_string_get_length (&message->header), + DBUS_COMPILER_BYTE_ORDER, len_data); } /** @@ -213,8 +227,9 @@ _dbus_message_lock (DBusMessage *message) * Constructs a new message. Returns #NULL if memory * can't be allocated for the message. * + * @todo use DBusString internally to store service and name. + * * @param service service that the message should be sent to - * should be sent to * @param name name of the message * @returns a new DBusMessage, free with dbus_message_unref() * @see dbus_message_unref() @@ -240,12 +255,16 @@ dbus_message_new (const char *service, if (!_dbus_string_init (&message->header, _DBUS_MAX_MESSAGE_LENGTH)) { + dbus_free (message->service); + dbus_free (message->name); dbus_free (message); return NULL; } if (!_dbus_string_init (&message->body, _DBUS_MAX_MESSAGE_LENGTH)) { + dbus_free (message->service); + dbus_free (message->name); _dbus_string_free (&message->header); dbus_free (message); return NULL; @@ -285,7 +304,8 @@ dbus_message_unref (DBusMessage *message) { _dbus_string_free (&message->header); _dbus_string_free (&message->body); - + + dbus_free (message->service); dbus_free (message->name); dbus_free (message); } @@ -310,18 +330,22 @@ dbus_message_get_name (DBusMessage *message) * The list is terminated with 0. * * @param message the message - * @param ... list of fields. + * @param type of the first field + * @param ... value of first field, list of additional type-value pairs * @returns #TRUE on success */ dbus_bool_t dbus_message_append_fields (DBusMessage *message, + int first_field_type, ...) { dbus_bool_t retval; va_list var_args; - va_start (var_args, message); - retval = dbus_message_append_fields_valist (message, var_args); + va_start (var_args, first_field_type); + retval = dbus_message_append_fields_valist (message, + first_field_type, + var_args); va_end (var_args); return retval; @@ -332,18 +356,20 @@ dbus_message_append_fields (DBusMessage *message, * * @see dbus_message_append_fields. * @param message the message - * @param var_args list of type/value pairs + * @param first_field_type type of first field + * @param var_args value of first field, then list of type/value pairs * @returns #TRUE on success */ dbus_bool_t dbus_message_append_fields_valist (DBusMessage *message, + int first_field_type, va_list var_args) { int type, old_len; old_len = _dbus_string_get_length (&message->body); - type = va_arg (var_args, int); + type = first_field_type; while (type != 0) { @@ -375,9 +401,8 @@ dbus_message_append_fields_valist (DBusMessage *message, if (!dbus_message_append_byte_array (message, data, len)) goto enomem; - - break; } + break; default: _dbus_warn ("Unknown field type %d\n", type); } @@ -516,18 +541,20 @@ dbus_message_append_byte_array (DBusMessage *message, * stored. The list is terminated with 0. * * @param message the message - * @param ... list of fields. + * @param first_field_type the first field type + * @param ... location for first field value, then list of type-location pairs * @returns #TRUE on success */ dbus_bool_t dbus_message_get_fields (DBusMessage *message, + int first_field_type, ...) { dbus_bool_t retval; va_list var_args; - va_start (var_args, message); - retval = dbus_message_get_fields_valist (message, var_args); + va_start (var_args, first_field_type); + retval = dbus_message_get_fields_valist (message, first_field_type, var_args); va_end (var_args); return retval; @@ -536,13 +563,22 @@ dbus_message_get_fields (DBusMessage *message, /** * This function takes a va_list for use by language bindings * + * @todo this function (or some lower-level non-convenience function) + * needs better error handling; should allow the application to + * distinguish between out of memory, and bad data from the remote + * app. It also needs to not leak a bunch of args when it gets + * to the arg that's bad, as that would be a security hole + * (allow one app to force another to leak memory) + * * @see dbus_message_get_fields * @param message the message - * @param var_args list of type/pointer pairs + * @param first_field_type type of the first field + * @param var_args return location for first field, followed by list of type/location pairs * @returns #TRUE on success */ dbus_bool_t dbus_message_get_fields_valist (DBusMessage *message, + int first_field_type, va_list var_args) { int spec_type, msg_type, i; @@ -553,7 +589,7 @@ dbus_message_get_fields_valist (DBusMessage *message, if (iter == NULL) return FALSE; - spec_type = va_arg (var_args, int); + spec_type = first_field_type; i = 0; while (spec_type != 0) @@ -632,7 +668,7 @@ dbus_message_get_fields_valist (DBusMessage *message, { _dbus_warn ("More fields than exist in the message were specified\n"); - dbus_message_iter_unref (iter); + dbus_message_iter_unref (iter); return FALSE; } i++; @@ -646,6 +682,12 @@ dbus_message_get_fields_valist (DBusMessage *message, * Returns a DBusMessageIter representing the fields of the * message passed in. * + * @todo IMO the message iter should follow the GtkTextIter pattern, + * a static object with a "stamp" value used to detect invalid + * iter uses (uninitialized or after changing the message). + * ref/unref is kind of annoying to deal with, and slower too. + * This implies not ref'ing the message from the iter. + * * @param message the message * @returns a new iter. */ @@ -783,7 +825,7 @@ dbus_message_iter_get_string (DBusMessageIter *iter) _dbus_assert (dbus_message_iter_get_field_type (iter) == DBUS_TYPE_STRING); return _dbus_demarshal_string (&iter->message->body, iter->message->byte_order, - iter->pos + 1, NULL); + iter->pos + 1, NULL); } /** @@ -987,6 +1029,31 @@ _dbus_message_loader_get_buffer (DBusMessageLoader *loader, */ #define DBUS_MINIMUM_HEADER_SIZE 16 +/** Pack four characters as in "abcd" into a uint32 */ +#define FOUR_CHARS_TO_UINT32(a, b, c, d) \ + ((((dbus_uint32_t)a) << 24) | \ + (((dbus_uint32_t)b) << 16) | \ + (((dbus_uint32_t)c) << 8) | \ + ((dbus_uint32_t)d)) + +/** DBUS_HEADER_FIELD_NAME packed into a dbus_uint32_t */ +#define DBUS_HEADER_FIELD_NAME_AS_UINT32 \ + FOUR_CHARS_TO_UINT32 ('n', 'a', 'm', 'e') + +/** DBUS_HEADER_FIELD_SERVICE packed into a dbus_uint32_t */ +#define DBUS_HEADER_FIELD_SERVICE_AS_UINT32 \ + FOUR_CHARS_TO_UINT32 ('s', 'r', 'v', 'c') + +/** DBUS_HEADER_FIELD_REPLY packed into a dbus_uint32_t */ +#define DBUS_HEADER_FIELD_REPLY_AS_UINT32 \ + FOUR_CHARS_TO_UINT32 ('r', 'p', 'l', 'y') + +/* FIXME should be using DBusString for the stuff we demarshal. char* + * evil. Also, out of memory handling here seems suboptimal. + * Should probably report it as a distinct error from "corrupt message," + * so we can postpone parsing this message. Also, we aren't + * checking for demarshal failure everywhere. + */ static dbus_bool_t decode_header_data (DBusString *data, int header_len, @@ -1007,27 +1074,49 @@ decode_header_data (DBusString *data, /* Now handle the fields */ while (pos < header_len) { + pos = _DBUS_ALIGN_VALUE (pos, 4); + + if ((pos + 4) > header_len) + return FALSE; + _dbus_string_get_const_data_len (data, &field, pos, 4); pos += 4; - if (pos > header_len) - return FALSE; - - if (strncmp (field, DBUS_HEADER_FIELD_SERVICE, 4) == 0) - { + _dbus_assert (_DBUS_ALIGN_ADDRESS (field, 4) == field); + + /* I believe FROM_BE is right, but if not we'll find out + * I guess. ;-) + */ + switch (DBUS_UINT32_FROM_BE (*(int*)field)) + { + case DBUS_HEADER_FIELD_SERVICE_AS_UINT32: + if (*service != NULL) + { + _dbus_verbose ("%s field provided twice\n", + DBUS_HEADER_FIELD_SERVICE); + goto failed; + } + *service = _dbus_demarshal_string (data, byte_order, pos + 1, &new_pos); - } - else if (strncmp (field, DBUS_HEADER_FIELD_NAME, 4) == 0) - { + /* FIXME check for demarshal failure SECURITY */ + break; + case DBUS_HEADER_FIELD_NAME_AS_UINT32: + if (*name != NULL) + { + _dbus_verbose ("%s field provided twice\n", + DBUS_HEADER_FIELD_NAME); + goto failed; + } + *name = _dbus_demarshal_string (data, byte_order, pos + 1, &new_pos); - } - else - { - _dbus_verbose ("Encountered an unknown header field: %c%c%c%c\n", + /* FIXME check for demarshal failure SECURITY */ + break; + default: + _dbus_verbose ("Ignoring an unknown header field: %c%c%c%c\n", field[0], field[1], field[2], field[3]); if (!_dbus_marshal_get_field_end_pos (data, byte_order, pos, &new_pos)) - return FALSE; + goto failed; } if (new_pos > header_len) @@ -1037,7 +1126,11 @@ decode_header_data (DBusString *data, } return TRUE; - + + failed: + dbus_free (*service); + dbus_free (*name); + return FALSE; } /** @@ -1070,22 +1163,28 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, int byte_order, header_len, body_len; _dbus_string_get_const_data_len (&loader->data, &header_data, 0, 16); + + _dbus_assert (_DBUS_ALIGN_ADDRESS (header_data, 4) == header_data); + byte_order = header_data[0]; if (byte_order != DBUS_LITTLE_ENDIAN && byte_order != DBUS_BIG_ENDIAN) { + _dbus_verbose ("Message with bad byte order '%c' received\n", + byte_order); loader->corrupted = TRUE; return; } - header_len = dbus_unpack_int32 (byte_order, header_data + 4); - body_len = dbus_unpack_int32 (byte_order, header_data + 8); + header_len = _dbus_unpack_int32 (byte_order, header_data + 4); + body_len = _dbus_unpack_int32 (byte_order, header_data + 8); if (header_len + body_len > _DBUS_MAX_MESSAGE_LENGTH) { + _dbus_verbose ("Message claimed length header = %d body = %d exceeds max message length %d\n", + header_len, body_len, _DBUS_MAX_MESSAGE_LENGTH); loader->corrupted = TRUE; - return; } @@ -1093,21 +1192,24 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader, { dbus_int32_t client_serial; char *service, *name; - - if (!decode_header_data (&loader->data, header_len, byte_order, + + /* FIXME right now if this doesn't have enough memory, the + * loader becomes corrupted. Instead we should just not + * parse this message for now. + */ + if (!decode_header_data (&loader->data, header_len, byte_order, &client_serial, &service, &name)) { loader->corrupted = TRUE; - return; } - message = dbus_message_new (service, name); - dbus_free (service); + message = dbus_message_new (service, name); + dbus_free (service); dbus_free (name); if (message == NULL) - break; /* ugh, postpone this I guess. */ + break; /* ugh, postpone this I guess. */ _dbus_string_copy (&loader->data, header_len, &message->body, 0); _dbus_message_set_client_serial (message, client_serial); diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index 10fb48a3..f1b49f9b 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -46,8 +46,10 @@ const char* dbus_message_get_name (DBusMessage *message); dbus_bool_t dbus_message_append_fields (DBusMessage *message, + int first_field_type, ...); dbus_bool_t dbus_message_append_fields_valist (DBusMessage *message, + int first_field_type, va_list var_args); dbus_bool_t dbus_message_append_int32 (DBusMessage *message, dbus_int32_t value); @@ -65,8 +67,10 @@ dbus_bool_t dbus_message_append_byte_array (DBusMessage *message, DBusMessageIter *dbus_message_get_fields_iter (DBusMessage *message); dbus_bool_t dbus_message_get_fields (DBusMessage *message, + int first_field_type, ...); dbus_bool_t dbus_message_get_fields_valist (DBusMessage *message, + int first_field_type, va_list var_args); void dbus_message_iter_ref (DBusMessageIter *iter); diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 6dc28577..c2f4150a 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -526,6 +526,41 @@ _dbus_string_set_length (DBusString *str, return set_length (real, length); } +/** + * Align the length of a string to a specific alignment (typically 4 or 8) + * by appending nul bytes to the string. + * + * @param str a string + * @param alignment the alignment + * @returns #FALSE if no memory + */ +dbus_bool_t +_dbus_string_align_length (DBusString *str, + int alignment) +{ + int new_len; + int delta; + DBUS_STRING_PREAMBLE (str); + _dbus_assert (alignment >= 1); + _dbus_assert (alignment <= 16); /* arbitrary */ + + new_len = _DBUS_ALIGN_VALUE (real->len, alignment); + + delta = new_len - real->len; + _dbus_assert (delta >= 0); + + if (delta == 0) + return TRUE; + + if (!set_length (real, new_len)) + return FALSE; + + memset (real->str + (new_len - delta), + '\0', delta); + + return TRUE; +} + static dbus_bool_t append (DBusRealString *real, const char *buffer, diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 471ac0c2..b2335086 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -72,12 +72,14 @@ dbus_bool_t _dbus_string_steal_data_len (DBusString *str, int _dbus_string_get_length (const DBusString *str); -dbus_bool_t _dbus_string_lengthen (DBusString *str, - int additional_length); -void _dbus_string_shorten (DBusString *str, - int length_to_remove); -dbus_bool_t _dbus_string_set_length (DBusString *str, - int length); +dbus_bool_t _dbus_string_lengthen (DBusString *str, + int additional_length); +void _dbus_string_shorten (DBusString *str, + int length_to_remove); +dbus_bool_t _dbus_string_set_length (DBusString *str, + int length); +dbus_bool_t _dbus_string_align_length (DBusString *str, + int alignment); dbus_bool_t _dbus_string_append (DBusString *str, const char *buffer); diff --git a/dbus/dbus-test-main.c b/dbus/dbus-test-main.c new file mode 100644 index 00000000..8ed77cce --- /dev/null +++ b/dbus/dbus-test-main.c @@ -0,0 +1,36 @@ +/* -*- mode: C; c-file-style: "gnu" -*- */ +/* dbus-test.c Program to run all tests + * + * Copyright (C) 2002 Red Hat Inc. + * + * Licensed under the Academic Free License version 1.2 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + + +#include "dbus-types.h" +#include "dbus-test.h" +#include <stdio.h> +#include <stdlib.h> + +int +main (int argc, + char **argv) +{ + dbus_internal_symbol_do_not_use_run_tests (); + return 0; +} diff --git a/dbus/dbus-test.c b/dbus/dbus-test.c index 2866e084..a219069f 100644 --- a/dbus/dbus-test.c +++ b/dbus/dbus-test.c @@ -21,7 +21,6 @@ * */ -#include "dbus-types.h" #include "dbus-test.h" #include <stdio.h> #include <stdlib.h> @@ -33,34 +32,41 @@ die (const char *failure) exit (1); } -int -main (int argc, - char **argv) +/** + * An exported symbol to be run in order to execute + * unit tests. Should not be used by + * any app other than our test app, this symbol + * won't exist in some builds of the library. + * (with --enable-tests=no) + */ +void +dbus_internal_symbol_do_not_use_run_tests (void) { - printf ("%s: running string tests\n", argv[0]); + printf ("%s: running string tests\n", "dbus-test"); if (!_dbus_string_test ()) die ("strings"); - printf ("%s: running marshalling tests\n", argv[0]); + printf ("%s: running marshalling tests\n", "dbus-test"); if (!_dbus_marshal_test ()) die ("marshalling"); - printf ("%s: running message tests\n", argv[0]); + printf ("%s: running message tests\n", "dbus-test"); if (!_dbus_message_test ()) die ("messages"); - printf ("%s: running memory pool tests\n", argv[0]); + printf ("%s: running memory pool tests\n", "dbus-test"); if (!_dbus_mem_pool_test ()) die ("memory pools"); - printf ("%s: running linked list tests\n", argv[0]); + printf ("%s: running linked list tests\n", "dbus-test"); if (!_dbus_list_test ()) die ("lists"); - printf ("%s: running hash table tests\n", argv[0]); + printf ("%s: running hash table tests\n", "dbus-test"); if (!_dbus_hash_test ()) die ("hash tables"); - printf ("%s: completed successfully\n", argv[0]); - return 0; + printf ("%s: completed successfully\n", "dbus-test"); } + + diff --git a/dbus/dbus-test.h b/dbus/dbus-test.h index 881d0e42..64faf12b 100644 --- a/dbus/dbus-test.h +++ b/dbus/dbus-test.h @@ -33,4 +33,6 @@ dbus_bool_t _dbus_mem_pool_test (void); dbus_bool_t _dbus_string_test (void); dbus_bool_t _dbus_message_test (void); +void dbus_internal_symbol_do_not_use_run_tests (void); + #endif /* DBUS_TEST_H */ |