From 606eb2106b3d7f5cd36a4f1786d281b771bb1bf7 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 21 Jan 2005 06:18:04 +0000 Subject: 2005-01-21 Havoc Pennington * dbus/dbus-bus.c: add more return_if_fail checks * dbus/dbus-message.c (load_message): have the "no validation" mode (have to edit the code to toggle the mode for now though) * dbus/dbus-marshal-header.c (_dbus_header_load): have a mode that skips all validation; I want to use this at least for benchmark baseline, I'm not sure if it should be a publicly-available switch. --- dbus/dbus-bus.c | 10 +++++++ dbus/dbus-marshal-header.c | 70 ++++++++++++++++++++++++++++---------------- dbus/dbus-marshal-header.h | 3 +- dbus/dbus-marshal-validate.h | 9 ++++++ dbus/dbus-message.c | 54 +++++++++++++++++++--------------- 5 files changed, 95 insertions(+), 51 deletions(-) (limited to 'dbus') diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index 68b69719..bc6a3bf5 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -564,6 +564,7 @@ dbus_bus_get_unix_user (DBusConnection *connection, _dbus_return_val_if_fail (connection != NULL, DBUS_UID_UNSET); _dbus_return_val_if_fail (name != NULL, DBUS_UID_UNSET); + _dbus_return_val_if_fail (_dbus_check_is_valid_bus_name (name), DBUS_UID_UNSET); _dbus_return_val_if_error_is_set (error, DBUS_UID_UNSET); message = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, @@ -645,6 +646,7 @@ dbus_bus_request_name (DBusConnection *connection, _dbus_return_val_if_fail (connection != NULL, 0); _dbus_return_val_if_fail (name != NULL, 0); + _dbus_return_val_if_fail (_dbus_check_is_valid_bus_name (name), 0); _dbus_return_val_if_error_is_set (error, 0); message = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, @@ -718,6 +720,7 @@ dbus_bus_name_has_owner (DBusConnection *connection, _dbus_return_val_if_fail (connection != NULL, FALSE); _dbus_return_val_if_fail (name != NULL, FALSE); + _dbus_return_val_if_fail (_dbus_check_is_valid_bus_name (name), FALSE); _dbus_return_val_if_error_is_set (error, FALSE); message = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, @@ -786,6 +789,9 @@ dbus_bus_start_service_by_name (DBusConnection *connection, DBusMessage *msg; DBusMessage *reply; + _dbus_return_val_if_fail (connection != NULL, FALSE); + _dbus_return_val_if_fail (_dbus_check_is_valid_bus_name (name), FALSE); + msg = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, DBUS_PATH_ORG_FREEDESKTOP_DBUS, DBUS_INTERFACE_ORG_FREEDESKTOP_DBUS, @@ -884,6 +890,8 @@ dbus_bus_add_match (DBusConnection *connection, { DBusMessage *msg; + _dbus_return_if_fail (rule != NULL); + msg = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, DBUS_PATH_ORG_FREEDESKTOP_DBUS, DBUS_INTERFACE_ORG_FREEDESKTOP_DBUS, @@ -928,6 +936,8 @@ dbus_bus_remove_match (DBusConnection *connection, { DBusMessage *msg; + _dbus_return_if_fail (rule != NULL); + msg = dbus_message_new_method_call (DBUS_SERVICE_ORG_FREEDESKTOP_DBUS, DBUS_PATH_ORG_FREEDESKTOP_DBUS, DBUS_INTERFACE_ORG_FREEDESKTOP_DBUS, diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c index 8346c3af..6102b6f0 100644 --- a/dbus/dbus-marshal-header.c +++ b/dbus/dbus-marshal-header.c @@ -917,12 +917,12 @@ load_and_validate_field (DBusHeader *header, } /** - * Creates a message header from 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. + * 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. * * The byte_order, fields_array_len, and body_len args should be from * _dbus_header_have_message_untrusted(). Validation performed in @@ -930,6 +930,7 @@ load_and_validate_field (DBusHeader *header, * already done. * * @param header the header (must be initialized) + * @param mode whether to do validation * @param validity return location for invalidity reason * @param byte_order byte order from header * @param fields_array_len claimed length of fields array @@ -941,15 +942,16 @@ load_and_validate_field (DBusHeader *header, * @returns #FALSE if no memory or data was invalid, #TRUE otherwise */ dbus_bool_t -_dbus_header_load_untrusted (DBusHeader *header, - DBusValidity *validity, - int byte_order, - int fields_array_len, - int header_len, - int body_len, - const DBusString *str, - int start, - int len) +_dbus_header_load (DBusHeader *header, + DBusValidationMode mode, + DBusValidity *validity, + int byte_order, + int fields_array_len, + int header_len, + int body_len, + const DBusString *str, + int start, + int len) { int leftover; DBusValidity v; @@ -973,15 +975,22 @@ _dbus_header_load_untrusted (DBusHeader *header, return FALSE; } - v = _dbus_validate_body_with_reason (&_dbus_header_signature_str, 0, - byte_order, - &leftover, - str, start, len); - - if (v != DBUS_VALID) + if (mode == DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY) { - *validity = v; - goto invalid; + leftover = len - header_len - body_len - start; + } + else + { + v = _dbus_validate_body_with_reason (&_dbus_header_signature_str, 0, + byte_order, + &leftover, + str, start, len); + + if (v != DBUS_VALID) + { + *validity = v; + goto invalid; + } } _dbus_assert (leftover < len); @@ -991,14 +1000,23 @@ _dbus_header_load_untrusted (DBusHeader *header, _dbus_assert (start + header_len == (int) _DBUS_ALIGN_VALUE (padding_start, 8)); _dbus_assert (start + header_len == padding_start + padding_len); - if (!_dbus_string_validate_nul (str, padding_start, padding_len)) + if (mode != DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY) { - *validity = DBUS_INVALID_ALIGNMENT_PADDING_NOT_NUL; - goto invalid; + if (!_dbus_string_validate_nul (str, padding_start, padding_len)) + { + *validity = DBUS_INVALID_ALIGNMENT_PADDING_NOT_NUL; + goto invalid; + } } header->padding = padding_len; + if (mode == DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY) + { + *validity = DBUS_VALID; + return TRUE; + } + /* We now know the data is well-formed, but we have to check that * it's valid. */ diff --git a/dbus/dbus-marshal-header.h b/dbus/dbus-marshal-header.h index 61e9246a..fed2888b 100644 --- a/dbus/dbus-marshal-header.h +++ b/dbus/dbus-marshal-header.h @@ -115,7 +115,8 @@ dbus_bool_t _dbus_header_have_message_untrusted (int max_messag const DBusString *str, int start, int len); -dbus_bool_t _dbus_header_load_untrusted (DBusHeader *header, +dbus_bool_t _dbus_header_load (DBusHeader *header, + DBusValidationMode mode, DBusValidity *validity, int byte_order, int fields_array_len, diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h index 6b37338b..21d2e9e3 100644 --- a/dbus/dbus-marshal-validate.h +++ b/dbus/dbus-marshal-validate.h @@ -31,6 +31,15 @@ #error "config.h not included here" #endif +/** + * This is used rather than a bool for high visibility + */ +typedef enum +{ + DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY, + DBUS_VALIDATION_MODE_DATA_IS_UNTRUSTED +} DBusValidationMode; + /** * This is primarily used in unit testing, so we can verify that each * invalid message is invalid for the expected reasons. Thus we really diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 4ddfa37a..2ed9cf2f 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -3032,7 +3032,10 @@ load_message (DBusMessageLoader *loader, DBusValidity validity; const DBusString *type_str; int type_pos; + DBusValidationMode mode; + mode = DBUS_VALIDATION_MODE_DATA_IS_UNTRUSTED; + oom = FALSE; #if 0 @@ -3043,14 +3046,15 @@ load_message (DBusMessageLoader *loader, _dbus_assert (_dbus_string_get_length (&message->header.data) == 0); _dbus_assert ((header_len + body_len) <= _dbus_string_get_length (&loader->data)); - if (!_dbus_header_load_untrusted (&message->header, - &validity, - byte_order, - fields_array_len, - header_len, - body_len, - &loader->data, 0, - _dbus_string_get_length (&loader->data))) + if (!_dbus_header_load (&message->header, + mode, + &validity, + byte_order, + fields_array_len, + header_len, + body_len, + &loader->data, 0, + _dbus_string_get_length (&loader->data))) { _dbus_verbose ("Failed to load header for new message code %d\n", validity); if (validity == DBUS_VALID) @@ -3063,23 +3067,25 @@ load_message (DBusMessageLoader *loader, message->byte_order = byte_order; /* 2. VALIDATE BODY */ - - get_const_signature (&message->header, &type_str, &type_pos); - - /* Because the bytes_remaining arg is NULL, this validates that the - * body is the right length - */ - validity = _dbus_validate_body_with_reason (type_str, - type_pos, - byte_order, - NULL, - &loader->data, - header_len, - body_len); - if (validity != DBUS_VALID) + if (mode != DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY) { - _dbus_verbose ("Failed to validate message body code %d\n", validity); - goto failed; + get_const_signature (&message->header, &type_str, &type_pos); + + /* Because the bytes_remaining arg is NULL, this validates that the + * body is the right length + */ + validity = _dbus_validate_body_with_reason (type_str, + type_pos, + byte_order, + NULL, + &loader->data, + header_len, + body_len); + if (validity != DBUS_VALID) + { + _dbus_verbose ("Failed to validate message body code %d\n", validity); + goto failed; + } } /* 3. COPY OVER BODY AND QUEUE MESSAGE */ -- cgit