From 5e389fdf499c39926c61b47fcafb5e71291ce1a2 Mon Sep 17 00:00:00 2001 From: "John (J5) Palmieri" Date: Wed, 15 Jun 2005 15:15:32 +0000 Subject: * dbus/dbus-marshal-validate.h: Added a new validation error code DBUS_VALIDITY_UNKNOWN_OOM_ERROR = -4 for out of memory errors when validating signitures * dbus/dbus-marshal-header.c: use DBUS_VALIDITY_UNKNOWN_OOM_ERROR in places where we previously used DBUS_VALID and a FALSE return value to indicate OOM * dbus/dbus-marshal-validate.c (_dbus_validate_signature_with_reason): Use a stack to track the number of elements inside containers. The stack values are then used to validate that dict entries have only two elements within them. (validate_body_helper): check the reason for failure when validating varients * dbus/dbus-message.c (load_message): use DBUS_VALIDITY_UNKNOWN_OOM_ERROR in places where we previously used DBUS_VALID and a FALSE return value to indicate OOM * doc/TODO: remove "- validate dict entry number of fields" as this patch fixes it --- ChangeLog | 24 ++++++ dbus/dbus-marshal-header.c | 9 ++- dbus/dbus-marshal-validate.c | 184 +++++++++++++++++++++++++++++++++++-------- dbus/dbus-marshal-validate.h | 3 +- dbus/dbus-message.c | 7 +- doc/TODO | 2 - 6 files changed, 188 insertions(+), 41 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7ca9efc0..6d3087e0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2005-06-15 John (J5) Palmieri + + * dbus/dbus-marshal-validate.h: Added a new validation + error code DBUS_VALIDITY_UNKNOWN_OOM_ERROR = -4 for + out of memory errors when validating signitures + + * dbus/dbus-marshal-header.c: use DBUS_VALIDITY_UNKNOWN_OOM_ERROR + in places where we previously used DBUS_VALID and a FALSE return + value to indicate OOM + + * dbus/dbus-marshal-validate.c (_dbus_validate_signature_with_reason): + Use a stack to track the number of elements inside containers. The + stack values are then used to validate that dict entries have only two + elements within them. + (validate_body_helper): check the reason for failure when validating + varients + + * dbus/dbus-message.c (load_message): use + DBUS_VALIDITY_UNKNOWN_OOM_ERROR in places where we previously used + DBUS_VALID and a FALSE return value to indicate OOM + + * doc/TODO: remove "- validate dict entry number of fields" as this + patch fixes it + 2005-06-14 David Zeuthen * bus/bus.c (process_config_every_time): Drop existing conf-dir diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c index 967a01e3..83fbd3bf 100644 --- a/dbus/dbus-marshal-header.c +++ b/dbus/dbus-marshal-header.c @@ -926,9 +926,10 @@ load_and_validate_field (DBusHeader *header, * Creates a message header from potentially-untrusted data. The * return value is #TRUE if there was enough memory and the data was * valid. If it returns #TRUE, the header will be created. If it - * returns #FALSE and *validity == #DBUS_VALID, then there wasn't - * enough memory. If it returns #FALSE and *validity != #DBUS_VALID - * then the data was invalid. + * returns #FALSE and *validity == #DBUS_VALIDITY_UNKNOWN_OOM_ERROR, + * then there wasn't enough memory. If it returns #FALSE + * and *validity != #DBUS_VALIDITY_UNKNOWN_OOM_ERROR then the data was + * invalid. * * The byte_order, fields_array_len, and body_len args should be from * _dbus_header_have_message_untrusted(). Validation performed in @@ -977,7 +978,7 @@ _dbus_header_load (DBusHeader *header, if (!_dbus_string_copy_len (str, start, header_len, &header->data, 0)) { _dbus_verbose ("Failed to copy buffer into new header\n"); - *validity = DBUS_VALID; + *validity = DBUS_VALIDITY_UNKNOWN_OOM_ERROR; return FALSE; } diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index dbe2fa54..92050a59 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -61,6 +61,19 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, int struct_depth; int array_depth; int dict_entry_depth; + DBusValidity result; + + int element_count; + DBusList *element_count_stack; + + result = DBUS_VALID; + element_count_stack = NULL; + + if (!_dbus_list_append (&element_count_stack, _DBUS_INT_TO_POINTER (0))) + { + result = DBUS_VALIDITY_UNKNOWN_OOM_ERROR; + goto out; + } _dbus_assert (type_str != NULL); _dbus_assert (type_pos < _DBUS_INT32_MAX - len); @@ -68,9 +81,13 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, _dbus_assert (type_pos >= 0); if (len > DBUS_MAXIMUM_SIGNATURE_LENGTH) - return DBUS_INVALID_SIGNATURE_TOO_LONG; + { + result = DBUS_INVALID_SIGNATURE_TOO_LONG; + goto out; + } p = _dbus_string_get_const_data_len (type_str, type_pos, 0); + end = _dbus_string_get_const_data_len (type_str, type_pos + len, 0); struct_depth = 0; array_depth = 0; @@ -99,85 +116,179 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, case DBUS_TYPE_ARRAY: array_depth += 1; if (array_depth > DBUS_MAXIMUM_TYPE_RECURSION_DEPTH) - return DBUS_INVALID_EXCEEDED_MAXIMUM_ARRAY_RECURSION; + { + result = DBUS_INVALID_EXCEEDED_MAXIMUM_ARRAY_RECURSION; + goto out; + } break; case DBUS_STRUCT_BEGIN_CHAR: struct_depth += 1; if (struct_depth > DBUS_MAXIMUM_TYPE_RECURSION_DEPTH) - return DBUS_INVALID_EXCEEDED_MAXIMUM_STRUCT_RECURSION; + { + result = DBUS_INVALID_EXCEEDED_MAXIMUM_STRUCT_RECURSION; + goto out; + } + + if (!_dbus_list_append (&element_count_stack, + _DBUS_INT_TO_POINTER (0))) + { + result = DBUS_VALIDITY_UNKNOWN_OOM_ERROR; + goto out; + } + break; case DBUS_STRUCT_END_CHAR: if (struct_depth == 0) - return DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED; + { + result = DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED; + goto out; + } if (last == DBUS_STRUCT_BEGIN_CHAR) - return DBUS_INVALID_STRUCT_HAS_NO_FIELDS; + { + result = DBUS_INVALID_STRUCT_HAS_NO_FIELDS; + goto out; + } + + _dbus_list_pop_last (&element_count_stack); struct_depth -= 1; break; case DBUS_DICT_ENTRY_BEGIN_CHAR: if (last != DBUS_TYPE_ARRAY) - return DBUS_INVALID_DICT_ENTRY_NOT_INSIDE_ARRAY; - + { + result = DBUS_INVALID_DICT_ENTRY_NOT_INSIDE_ARRAY; + goto out; + } + dict_entry_depth += 1; if (dict_entry_depth > DBUS_MAXIMUM_TYPE_RECURSION_DEPTH) - return DBUS_INVALID_EXCEEDED_MAXIMUM_DICT_ENTRY_RECURSION; + { + result = DBUS_INVALID_EXCEEDED_MAXIMUM_DICT_ENTRY_RECURSION; + goto out; + } + + if (!_dbus_list_append (&element_count_stack, + _DBUS_INT_TO_POINTER (0))) + { + result = DBUS_VALIDITY_UNKNOWN_OOM_ERROR; + goto out; + } + break; case DBUS_DICT_ENTRY_END_CHAR: if (dict_entry_depth == 0) - return DBUS_INVALID_DICT_ENTRY_ENDED_BUT_NOT_STARTED; - - if (last == DBUS_DICT_ENTRY_BEGIN_CHAR) - return DBUS_INVALID_DICT_ENTRY_HAS_NO_FIELDS; - + { + result = DBUS_INVALID_DICT_ENTRY_ENDED_BUT_NOT_STARTED; + goto out; + } + dict_entry_depth -= 1; + + element_count = + _DBUS_POINTER_TO_INT (_dbus_list_pop_last (&element_count_stack)); + + if (element_count != 2) + { + if (element_count == 0) + result = DBUS_INVALID_DICT_ENTRY_HAS_NO_FIELDS; + else if (element_count == 1) + result = DBUS_INVALID_DICT_ENTRY_HAS_ONLY_ONE_FIELD; + else + result = DBUS_INVALID_DICT_ENTRY_HAS_TOO_MANY_FIELDS; + + goto out; + } break; case DBUS_TYPE_STRUCT: /* doesn't appear in signatures */ case DBUS_TYPE_DICT_ENTRY: /* ditto */ default: - return DBUS_INVALID_UNKNOWN_TYPECODE; + result = DBUS_INVALID_UNKNOWN_TYPECODE; + goto out; } + if (*p != DBUS_TYPE_ARRAY && + *p != DBUS_DICT_ENTRY_BEGIN_CHAR && + *p != DBUS_STRUCT_BEGIN_CHAR) + { + element_count = + _DBUS_POINTER_TO_INT (_dbus_list_pop_last (&element_count_stack)); + + ++element_count; + + if (!_dbus_list_append (&element_count_stack, + _DBUS_INT_TO_POINTER (element_count))) + { + result = DBUS_VALIDITY_UNKNOWN_OOM_ERROR; + goto out; + } + } + 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; + if (*p == DBUS_TYPE_ARRAY && p != end) + { + const char *p1; + p1 = p + 1; + if (*p1 == DBUS_STRUCT_END_CHAR || + *p1 == DBUS_DICT_ENTRY_END_CHAR) + { + result = DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE; + goto out; + } + } else - array_depth = 0; + { + array_depth = 0; + } } if (last == DBUS_DICT_ENTRY_BEGIN_CHAR && !dbus_type_is_basic (*p)) - return DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE; - + { + result = DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE; + goto out; + } + last = *p; ++p; } - if (array_depth > 0) - return DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE; + if (array_depth > 0) + { + result = DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE; + goto out; + } + if (struct_depth > 0) - return DBUS_INVALID_STRUCT_STARTED_BUT_NOT_ENDED; - + { + result = DBUS_INVALID_STRUCT_STARTED_BUT_NOT_ENDED; + goto out; + } + if (dict_entry_depth > 0) - return DBUS_INVALID_DICT_ENTRY_STARTED_BUT_NOT_ENDED; - + { + result = DBUS_INVALID_DICT_ENTRY_STARTED_BUT_NOT_ENDED; + goto out; + } + _dbus_assert (last != DBUS_TYPE_ARRAY); _dbus_assert (last != DBUS_STRUCT_BEGIN_CHAR); _dbus_assert (last != DBUS_DICT_ENTRY_BEGIN_CHAR); - - return DBUS_VALID; + + result = DBUS_VALID; + +out: + _dbus_list_clear (&element_count_stack); + return result; } static DBusValidity @@ -387,6 +498,7 @@ validate_body_helper (DBusTypeReader *reader, DBusValidity validity; int contained_alignment; int contained_type; + DBusValidity reason; claimed_len = *p; ++p; @@ -396,9 +508,15 @@ validate_body_helper (DBusTypeReader *reader, return DBUS_INVALID_VARIANT_SIGNATURE_LENGTH_OUT_OF_BOUNDS; _dbus_string_init_const_len (&sig, p, claimed_len); - if (!_dbus_validate_signature (&sig, 0, - _dbus_string_get_length (&sig))) - return DBUS_INVALID_VARIANT_SIGNATURE_BAD; + reason = _dbus_validate_signature_with_reason (&sig, 0, + _dbus_string_get_length (&sig)); + if (!(reason == DBUS_VALID)) + { + if (reason == DBUS_VALIDITY_UNKNOWN_OOM_ERROR) + return reason; + else + return DBUS_INVALID_VARIANT_SIGNATURE_BAD; + } p += claimed_len; diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h index 29a528b5..0074c437 100644 --- a/dbus/dbus-marshal-validate.h +++ b/dbus/dbus-marshal-validate.h @@ -48,7 +48,8 @@ typedef enum */ typedef enum { -#define _DBUS_NEGATIVE_VALIDITY_COUNT 3 +#define _DBUS_NEGATIVE_VALIDITY_COUNT 4 + DBUS_VALIDITY_UNKNOWN_OOM_ERROR = -4, DBUS_INVALID_FOR_UNKNOWN_REASON = -3, DBUS_VALID_BUT_INCOMPLETE = -2, DBUS_VALIDITY_UNKNOWN = -1, diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index d9a5234f..cdfdf5f3 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -3205,7 +3205,12 @@ load_message (DBusMessageLoader *loader, _dbus_string_get_length (&loader->data))) { _dbus_verbose ("Failed to load header for new message code %d\n", validity); - if (validity == DBUS_VALID) + + /* assert here so we can catch any code that still uses DBUS_VALID to indicate + oom errors. They should use DBUS_VALIDITY_UNKNOWN_OOM_ERROR instead */ + _dbus_assert (validity != DBUS_VALID); + + if (validity == DBUS_VALIDITY_UNKNOWN_OOM_ERROR) oom = TRUE; else { diff --git a/doc/TODO b/doc/TODO index 659dde15..773ebf46 100644 --- a/doc/TODO +++ b/doc/TODO @@ -22,8 +22,6 @@ Important for 1.0 yourself; is it an error, or allowed? If allowed, we need to have a test for it in the test suite. - - validate dict entry number of fields - - just before 1.0, try a HAVE_INT64=0 build and be sure it runs - in dbus-keyring.c, enforce that the keyring dir is not -- cgit