diff options
| author | John (J5) Palmieri <johnp@redhat.com> | 2005-06-15 15:15:32 +0000 | 
|---|---|---|
| committer | John (J5) Palmieri <johnp@redhat.com> | 2005-06-15 15:15:32 +0000 | 
| commit | 5e389fdf499c39926c61b47fcafb5e71291ce1a2 (patch) | |
| tree | b542e635c28d230c1aab61c8bca05f9f8041a18a | |
| parent | 1d19fc62e9034cc5700c0903f68787a84f485315 (diff) | |
	* 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
| -rw-r--r-- | ChangeLog | 24 | ||||
| -rw-r--r-- | dbus/dbus-marshal-header.c | 9 | ||||
| -rw-r--r-- | dbus/dbus-marshal-validate.c | 184 | ||||
| -rw-r--r-- | dbus/dbus-marshal-validate.h | 3 | ||||
| -rw-r--r-- | dbus/dbus-message.c | 7 | ||||
| -rw-r--r-- | doc/TODO | 2 | 
6 files changed, 188 insertions, 41 deletions
@@ -1,3 +1,27 @@ +2005-06-15  John (J5) Palmieri  <johnp@redhat.com> + +	* 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  <davidz@redhat.com>  	* 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          { @@ -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   | 
