summaryrefslogtreecommitdiffstats
path: root/dbus
diff options
context:
space:
mode:
authorHavoc Pennington <hp@redhat.com>2003-01-30 04:20:44 +0000
committerHavoc Pennington <hp@redhat.com>2003-01-30 04:20:44 +0000
commit7ba714ad7fe8256edfaad7d9a0f09aeb9611ca44 (patch)
tree921d4ee8a780d5fe03f168405a5811287c10a926 /dbus
parent8fdd8915bd7424cdf90bf59a018838a1290ac0c4 (diff)
2003-01-30 Havoc Pennington <hp@pobox.com>
* dbus/dbus-message.c: use message->byte_order instead of DBUS_COMPILER_BYTE_ORDER throughout. (dbus_message_create_header): pad header to align the start of the body of the message to 8-byte boundary * dbus/dbus-marshal.h: make all the demarshalers take const DBusString arguments. * dbus/dbus-message.c (_dbus_message_loader_return_buffer): validate message args here, so we don't have to do slow validation later, and so we catch bad messages as they are incoming. Also add better checks on header_len and body_len. Also fill in message->byte_order * dbus/dbus-string.c (_dbus_string_validate_utf8): new (not implemented properly) (_dbus_string_validate_nul): new function to check all-nul * dbus/dbus-marshal.c (_dbus_marshal_get_field_end_pos): rename get_arg_end_pos and remove all validation (_dbus_marshal_validate_arg): actually do validation here.
Diffstat (limited to 'dbus')
-rw-r--r--dbus/dbus-marshal.c371
-rw-r--r--dbus/dbus-marshal.h99
-rw-r--r--dbus/dbus-message.c191
-rw-r--r--dbus/dbus-string.c61
-rw-r--r--dbus/dbus-string.h8
5 files changed, 587 insertions, 143 deletions
diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c
index 9c17aabc..efdc7efb 100644
--- a/dbus/dbus-marshal.c
+++ b/dbus/dbus-marshal.c
@@ -73,7 +73,7 @@ _dbus_unpack_uint32 (int byte_order,
return DBUS_UINT32_FROM_LE (*(dbus_uint32_t*)data);
else
return DBUS_UINT32_FROM_BE (*(dbus_uint32_t*)data);
-}
+}
/**
* Unpacks a 32 bit signed integer from a data pointer
@@ -504,7 +504,7 @@ _dbus_marshal_string_array (DBusString *str,
* @returns the demarshaled double.
*/
double
-_dbus_demarshal_double (DBusString *str,
+_dbus_demarshal_double (const DBusString *str,
int byte_order,
int pos,
int *new_pos)
@@ -537,7 +537,7 @@ _dbus_demarshal_double (DBusString *str,
* @returns the demarshaled integer.
*/
dbus_int32_t
-_dbus_demarshal_int32 (DBusString *str,
+_dbus_demarshal_int32 (const DBusString *str,
int byte_order,
int pos,
int *new_pos)
@@ -564,7 +564,7 @@ _dbus_demarshal_int32 (DBusString *str,
* @returns the demarshaled integer.
*/
dbus_uint32_t
-_dbus_demarshal_uint32 (DBusString *str,
+_dbus_demarshal_uint32 (const DBusString *str,
int byte_order,
int pos,
int *new_pos)
@@ -598,7 +598,7 @@ _dbus_demarshal_uint32 (DBusString *str,
* @returns the demarshaled string.
*/
char *
-_dbus_demarshal_string (DBusString *str,
+_dbus_demarshal_string (const DBusString *str,
int byte_order,
int pos,
int *new_pos)
@@ -606,7 +606,7 @@ _dbus_demarshal_string (DBusString *str,
int len;
char *retval;
const char *data;
-
+
len = _dbus_demarshal_uint32 (str, byte_order, pos, &pos);
retval = dbus_malloc (len + 1);
@@ -641,7 +641,7 @@ _dbus_demarshal_string (DBusString *str,
* @returns the demarshaled data.
*/
unsigned char *
-_dbus_demarshal_byte_array (DBusString *str,
+_dbus_demarshal_byte_array (const DBusString *str,
int byte_order,
int pos,
int *new_pos,
@@ -685,7 +685,7 @@ _dbus_demarshal_byte_array (DBusString *str,
* @returns the demarshaled data.
*/
dbus_int32_t *
-_dbus_demarshal_int32_array (DBusString *str,
+_dbus_demarshal_int32_array (const DBusString *str,
int byte_order,
int pos,
int *new_pos,
@@ -724,7 +724,7 @@ _dbus_demarshal_int32_array (DBusString *str,
* @returns the demarshaled data.
*/
dbus_uint32_t *
-_dbus_demarshal_uint32_array (DBusString *str,
+_dbus_demarshal_uint32_array (const DBusString *str,
int byte_order,
int pos,
int *new_pos,
@@ -763,7 +763,7 @@ _dbus_demarshal_uint32_array (DBusString *str,
* @returns the demarshaled data.
*/
double *
-_dbus_demarshal_double_array (DBusString *str,
+_dbus_demarshal_double_array (const DBusString *str,
int byte_order,
int pos,
int *new_pos,
@@ -802,7 +802,7 @@ _dbus_demarshal_double_array (DBusString *str,
* @returns the demarshaled data.
*/
char **
-_dbus_demarshal_string_array (DBusString *str,
+_dbus_demarshal_string_array (const DBusString *str,
int byte_order,
int pos,
int *new_pos,
@@ -843,32 +843,24 @@ _dbus_demarshal_string_array (DBusString *str,
}
/**
- * Returns the position right after the end position
- * end position of a field. Validates the field
- * contents as required (e.g. ensures that
- * string fields have a valid length and
- * are nul-terminated).
+ * Returns the position right after the end of an argument. PERFORMS
+ * NO VALIDATION WHATSOEVER. The message must have been previously
+ * validated.
*
- * @todo security: audit the field validation code.
- *
- * @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.
+ * @todo handle DBUS_TYPE_NIL
*
* @param str a string
* @param byte_order the byte order to use
- * @param pos the pos where the field starts
+ * @param pos the pos where the arg starts
* @param end_pos pointer where the position right
* after the end position will follow
- * @returns TRUE if more data exists after the field
+ * @returns TRUE if more data exists after the arg
*/
dbus_bool_t
-_dbus_marshal_get_field_end_pos (DBusString *str,
- int byte_order,
- int pos,
- int *end_pos)
+_dbus_marshal_get_arg_end_pos (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *end_pos)
{
const char *data;
@@ -906,21 +898,8 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
len = _dbus_demarshal_uint32 (str, byte_order, pos + 1, &pos);
*end_pos = pos + len + 1;
-
- if (*end_pos > _dbus_string_get_length (str))
- {
- _dbus_verbose ("string length outside length of the message\n");
- return FALSE;
- }
-
- if (_dbus_string_get_byte (str, pos+len) != '\0')
- {
- _dbus_verbose ("string field not nul-terminated\n");
- return FALSE;
- }
-
- break;
}
+ break;
case DBUS_TYPE_BYTE_ARRAY:
{
@@ -930,9 +909,8 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
len = _dbus_demarshal_uint32 (str, byte_order, pos + 1, &pos);
*end_pos = pos + len;
-
- break;
}
+ break;
case DBUS_TYPE_INT32_ARRAY:
{
@@ -943,9 +921,8 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
*end_pos = _DBUS_ALIGN_VALUE (new_pos, sizeof (dbus_int32_t))
+ (len * sizeof (dbus_int32_t));
-
- break;
}
+ break;
case DBUS_TYPE_UINT32_ARRAY:
{
@@ -956,9 +933,8 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
*end_pos = _DBUS_ALIGN_VALUE (new_pos, sizeof (dbus_uint32_t))
+ (len * sizeof (dbus_uint32_t));
-
- break;
}
+ break;
case DBUS_TYPE_DOUBLE_ARRAY:
{
@@ -969,9 +945,8 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
*end_pos = _DBUS_ALIGN_VALUE (new_pos, sizeof (double))
+ (len * sizeof (double));
-
- break;
}
+ break;
case DBUS_TYPE_STRING_ARRAY:
{
@@ -990,10 +965,262 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
}
*end_pos = pos;
- break;
- }
+ }
+ break;
+
+ default:
+ _dbus_warn ("Unknown message arg type %d\n", *data);
+ _dbus_assert_not_reached ("Unknown message argument type\n");
+ return FALSE;
+ }
+
+ if (*end_pos > _dbus_string_get_length (str))
+ return FALSE;
+
+ return TRUE;
+}
+
+/**
+ * Demarshals and validates a length; returns < 0 if the validation
+ * fails. The length is required to be small enough that
+ * len*sizeof(double) will not overflow, and small enough to fit in a
+ * signed integer.
+ *
+ * @param str the string
+ * @param byte_order the byte order
+ * @param pos the unaligned string position (snap to next aligned)
+ */
+static int
+demarshal_and_validate_len (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos)
+{
+ int align_4 = _DBUS_ALIGN_VALUE (pos, 4);
+ unsigned int len;
+
+ if ((align_4 + 4) >= _dbus_string_get_length (str))
+ {
+ _dbus_verbose ("not enough room in message for array length\n");
+ return -1;
+ }
+
+ if (!_dbus_string_validate_nul (str, pos,
+ align_4 - pos))
+ {
+ _dbus_verbose ("array length alignment padding not initialized to nul\n");
+ return -1;
+ }
+
+ len = _dbus_demarshal_uint32 (str, byte_order, align_4, new_pos);
+
+ /* note that the len may be a number of doubles, so we need it to be
+ * at least SIZE_T_MAX / 8, but make it smaller just to keep things
+ * sane. We end up using ints for most sizes to avoid unsigned mess
+ * so limit to maximum 32-bit signed int divided by at least 8, more
+ * for a bit of paranoia margin. INT_MAX/32 is about 65 megabytes.
+ */
+#define MAX_ARRAY_LENGTH (((unsigned int)_DBUS_INT_MAX) / 32)
+ if (len > MAX_ARRAY_LENGTH)
+ {
+ _dbus_verbose ("array length %u exceeds maximum of %u\n",
+ len, MAX_ARRAY_LENGTH);
+ return -1;
+ }
+ else
+ return (int) len;
+}
+
+static dbus_bool_t
+validate_string (const DBusString *str,
+ int pos,
+ int len_without_nul,
+ int *end_pos)
+{
+ *end_pos = pos + len_without_nul + 1;
+
+ if (*end_pos > _dbus_string_get_length (str))
+ {
+ _dbus_verbose ("string length outside length of the message\n");
+ return FALSE;
+ }
+
+ if (_dbus_string_get_byte (str, pos + len_without_nul) != '\0')
+ {
+ _dbus_verbose ("string arg not nul-terminated\n");
+ return FALSE;
+ }
+
+ if (!_dbus_string_validate_utf8 (str, pos, len_without_nul))
+ {
+ _dbus_verbose ("string is not valid UTF-8\n");
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+/**
+ * Validates an argument, checking that it is well-formed, for example
+ * no ludicrous length fields, strings are nul-terminated, etc.
+ * Returns the end position of the argument in end_pos, and
+ * returns #TRUE if a valid arg begins at "pos"
+ *
+ * @todo security: need to audit this function.
+ *
+ * @todo handle DBUS_TYPE_NIL
+ *
+ * @param str a string
+ * @param byte_order the byte order to use
+ * @param pos the pos where the arg starts (offset of its typecode)
+ * @param end_pos pointer where the position right
+ * after the end position will follow
+ * @returns #TRUE if the arg is valid.
+ */
+dbus_bool_t
+_dbus_marshal_validate_arg (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *end_pos)
+{
+ const char *data;
+
+ if (pos >= _dbus_string_get_length (str))
+ return FALSE;
+
+ _dbus_string_get_const_data_len (str, &data, pos, 1);
+
+ switch (*data)
+ {
+ case DBUS_TYPE_INVALID:
+ return FALSE;
+ break;
+
+ case DBUS_TYPE_INT32:
+ case DBUS_TYPE_UINT32:
+ {
+ int align_4 = _DBUS_ALIGN_VALUE (pos + 1, 4);
+
+ if (!_dbus_string_validate_nul (str, pos + 1,
+ align_4 - pos - 1))
+ {
+ _dbus_verbose ("int32/uint32 alignment padding not initialized to nul\n");
+ return FALSE;
+ }
+
+ *end_pos = align_4 + 4;
+ }
+ break;
+
+ case DBUS_TYPE_DOUBLE:
+ {
+ int align_8 = _DBUS_ALIGN_VALUE (pos + 1, 8);
+
+ _dbus_verbose_bytes_of_string (str, pos, (align_8 + 8 - pos));
+
+ if (!_dbus_string_validate_nul (str, pos + 1,
+ align_8 - pos - 1))
+ {
+ _dbus_verbose ("double alignment padding not initialized to nul\n");
+ return FALSE;
+ }
+
+ *end_pos = align_8 + 8;
+ }
+ break;
+
+ case DBUS_TYPE_STRING:
+ {
+ int len;
+
+ /* Demarshal the length, which does NOT include
+ * nul termination
+ */
+ len = demarshal_and_validate_len (str, byte_order, pos + 1, &pos);
+ if (len < 0)
+ return FALSE;
+
+ if (!validate_string (str, pos, len, end_pos))
+ return FALSE;
+ }
+ break;
+
+ case DBUS_TYPE_BYTE_ARRAY:
+ {
+ int len;
+
+ len = demarshal_and_validate_len (str, byte_order, pos + 1, &pos);
+ if (len < 0)
+ return FALSE;
+
+ *end_pos = pos + len;
+ }
+ break;
+
+ case DBUS_TYPE_INT32_ARRAY:
+ case DBUS_TYPE_UINT32_ARRAY:
+ {
+ int len;
+
+ len = demarshal_and_validate_len (str, byte_order, pos + 1, &pos);
+ if (len < 0)
+ return FALSE;
+
+ _dbus_assert (_DBUS_ALIGN_VALUE (pos, 4) == (unsigned int) pos);
+
+ *end_pos = pos + len * 4;
+ }
+ break;
+
+ case DBUS_TYPE_DOUBLE_ARRAY:
+ {
+ int len;
+ int align_8;
+
+ len = demarshal_and_validate_len (str, byte_order, pos + 1, &pos);
+ if (len < 0)
+ return FALSE;
+
+ align_8 = _DBUS_ALIGN_VALUE (pos, 8);
+ if (!_dbus_string_validate_nul (str, pos,
+ align_8 - pos))
+ {
+ _dbus_verbose ("double array alignment padding not initialized to nul\n");
+ return FALSE;
+ }
+
+ *end_pos = align_8 + len * 8;
+ }
+ break;
+
+ case DBUS_TYPE_STRING_ARRAY:
+ {
+ int len;
+ int i;
+
+ len = demarshal_and_validate_len (str, byte_order, pos + 1, &pos);
+ if (len < 0)
+ return FALSE;
+
+ for (i = 0; i < len; i++)
+ {
+ int str_len;
+
+ str_len = demarshal_and_validate_len (str, byte_order,
+ pos, &pos);
+ if (str_len < 0)
+ return FALSE;
+
+ if (!validate_string (str, pos, str_len, &pos))
+ return FALSE;
+ }
+
+ *end_pos = pos;
+ }
+ break;
+
default:
- _dbus_warn ("Unknown message field type %d\n", *data);
+ _dbus_warn ("Unknown message arg type %d\n", *data);
return FALSE;
}
@@ -1003,6 +1230,7 @@ _dbus_marshal_get_field_end_pos (DBusString *str,
return TRUE;
}
+
/**
* If in verbose mode, print a block of binary data.
*
@@ -1018,6 +1246,8 @@ _dbus_verbose_bytes (const unsigned char *data,
int i;
const unsigned char *aligned;
+ _dbus_assert (len >= 0);
+
/* Print blanks on first row if appropriate */
aligned = _DBUS_ALIGN_ADDRESS (data, 4);
if (aligned > data)
@@ -1026,7 +1256,7 @@ _dbus_verbose_bytes (const unsigned char *data,
if (aligned != data)
{
- _dbus_verbose ("%5d\t%p: ", - (data - aligned), aligned);
+ _dbus_verbose ("%4d\t%p: ", - (data - aligned), aligned);
while (aligned != data)
{
_dbus_verbose (" ");
@@ -1040,7 +1270,7 @@ _dbus_verbose_bytes (const unsigned char *data,
{
if (_DBUS_ALIGN_ADDRESS (&data[i], 4) == &data[i])
{
- _dbus_verbose ("%5d\t%p: ",
+ _dbus_verbose ("%4d\t%p: ",
i, &data[i]);
}
@@ -1056,9 +1286,16 @@ _dbus_verbose_bytes (const unsigned char *data,
if (_DBUS_ALIGN_ADDRESS (&data[i], 4) == &data[i])
{
if (i > 3)
- _dbus_verbose ("big: %d little: %d",
+ _dbus_verbose ("BE: %d LE: %d",
_dbus_unpack_uint32 (DBUS_BIG_ENDIAN, &data[i-4]),
_dbus_unpack_uint32 (DBUS_LITTLE_ENDIAN, &data[i-4]));
+
+ if (i > 7 &&
+ _DBUS_ALIGN_ADDRESS (&data[i], 8) == &data[i])
+ {
+ _dbus_verbose (" dbl: %g",
+ *(double*)&data[i-8]);
+ }
_dbus_verbose ("\n");
}
@@ -1080,7 +1317,27 @@ _dbus_verbose_bytes_of_string (const DBusString *str,
int len)
{
const char *d;
+ int real_len;
+
+ real_len = _dbus_string_get_length (str);
+ _dbus_assert (start >= 0);
+
+ if (start > real_len)
+ {
+ _dbus_verbose (" [%d,%d) is not inside string of length %d\n",
+ start, len, real_len);
+ return;
+ }
+
+ if ((start + len) > real_len)
+ {
+ _dbus_verbose (" [%d,%d) extends outside string of length %d\n",
+ start, len, real_len);
+ len = real_len - start;
+ }
+
+
_dbus_string_get_const_data_len (str, &d, start, len);
_dbus_verbose_bytes (d, len);
diff --git a/dbus/dbus-marshal.h b/dbus/dbus-marshal.h
index bdeeccb9..cedca6b8 100644
--- a/dbus/dbus-marshal.h
+++ b/dbus/dbus-marshal.h
@@ -124,54 +124,57 @@ dbus_bool_t _dbus_marshal_string_array (DBusString *str,
const char **value,
int len);
-double _dbus_demarshal_double (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos);
-dbus_int32_t _dbus_demarshal_int32 (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos);
-dbus_uint32_t _dbus_demarshal_uint32 (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos);
-char * _dbus_demarshal_string (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos);
-unsigned char *_dbus_demarshal_byte_array (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos,
- int *array_len);
-dbus_int32_t *_dbus_demarshal_int32_array (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos,
- int *array_len);
-dbus_uint32_t *_dbus_demarshal_uint32_array (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos,
- int *array_len);
-double *_dbus_demarshal_double_array (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos,
- int *array_len);
-char **_dbus_demarshal_string_array (DBusString *str,
- int byte_order,
- int pos,
- int *new_pos,
- int *array_len);
-
-dbus_bool_t _dbus_marshal_get_field_end_pos (DBusString *str,
- int byte_order,
- int pos,
- int *end_pos);
-
-
+double _dbus_demarshal_double (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos);
+dbus_int32_t _dbus_demarshal_int32 (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos);
+dbus_uint32_t _dbus_demarshal_uint32 (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos);
+char * _dbus_demarshal_string (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos);
+unsigned char *_dbus_demarshal_byte_array (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos,
+ int *array_len);
+dbus_int32_t * _dbus_demarshal_int32_array (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos,
+ int *array_len);
+dbus_uint32_t *_dbus_demarshal_uint32_array (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos,
+ int *array_len);
+double * _dbus_demarshal_double_array (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos,
+ int *array_len);
+char ** _dbus_demarshal_string_array (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *new_pos,
+ int *array_len);
+
+
+dbus_bool_t _dbus_marshal_get_arg_end_pos (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *end_pos);
+dbus_bool_t _dbus_marshal_validate_arg (const DBusString *str,
+ int byte_order,
+ int pos,
+ int *end_pos);
#endif /* DBUS_PROTOCOL_H */
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index b1dd8564..85b49cbe 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -91,6 +91,7 @@ struct DBusMessage
HeaderField header_fields[FIELD_LAST]; /**< Track the location
* of each field in "header"
*/
+ int header_padding; /**< bytes of alignment in header */
DBusString body; /**< Body network data. */
@@ -138,6 +139,27 @@ _dbus_message_get_network_data (DBusMessage *message,
}
static void
+clear_header_padding (DBusMessage *message)
+{
+ _dbus_string_shorten (&message->header,
+ message->header_padding);
+ message->header_padding = 0;
+}
+
+static dbus_bool_t
+append_header_padding (DBusMessage *message)
+{
+ int old_len;
+ old_len = _dbus_string_get_length (&message->header);
+ if (!_dbus_string_align_length (&message->header, 8))
+ return FALSE;
+
+ message->header_padding = _dbus_string_get_length (&message->header) - old_len;
+
+ return TRUE;
+}
+
+static void
adjust_field_offsets (DBusMessage *message,
int offsets_after,
int delta)
@@ -209,6 +231,8 @@ append_int_field (DBusMessage *message,
int orig_len;
_dbus_assert (!message->locked);
+
+ clear_header_padding (message);
orig_len = _dbus_string_get_length (&message->header);
@@ -227,15 +251,24 @@ append_int_field (DBusMessage *message,
message->header_fields[FIELD_REPLY_SERIAL].offset =
_dbus_string_get_length (&message->header);
- if (!_dbus_marshal_int32 (&message->header, DBUS_COMPILER_BYTE_ORDER,
+ if (!_dbus_marshal_int32 (&message->header, message->byte_order,
value))
goto failed;
+ if (!append_header_padding (message))
+ goto failed;
+
return TRUE;
failed:
message->header_fields[field].offset = -1;
_dbus_string_set_length (&message->header, orig_len);
+
+ /* this must succeed because it was allocated on function entry and
+ * DBusString doesn't ever realloc smaller
+ */
+ if (!append_header_padding (message))
+ _dbus_assert_not_reached ("failed to reappend header padding");
return FALSE;
}
@@ -248,6 +281,8 @@ append_string_field (DBusMessage *message,
int orig_len;
_dbus_assert (!message->locked);
+
+ clear_header_padding (message);
orig_len = _dbus_string_get_length (&message->header);
@@ -266,15 +301,25 @@ append_string_field (DBusMessage *message,
message->header_fields[field].offset =
_dbus_string_get_length (&message->header);
- if (!_dbus_marshal_string (&message->header, DBUS_COMPILER_BYTE_ORDER,
+ if (!_dbus_marshal_string (&message->header, message->byte_order,
value))
goto failed;
+ if (!append_header_padding (message))
+ goto failed;
+
return TRUE;
failed:
message->header_fields[field].offset = -1;
_dbus_string_set_length (&message->header, orig_len);
+
+ /* this must succeed because it was allocated on function entry and
+ * DBusString doesn't ever realloc smaller
+ */
+ if (!append_header_padding (message))
+ _dbus_assert_not_reached ("failed to reappend header padding");
+
return FALSE;
}
@@ -290,6 +335,8 @@ delete_int_field (DBusMessage *message,
if (offset < 0)
return;
+ clear_header_padding (message);
+
/* The field typecode and name take up 8 bytes */
_dbus_string_delete (&message->header,
offset - 8,
@@ -300,6 +347,8 @@ delete_int_field (DBusMessage *message,
adjust_field_offsets (message,
offset - 8,
- 12);
+
+ append_header_padding (message);
}
static void
@@ -316,6 +365,8 @@ delete_string_field (DBusMessage *message,
if (offset < 0)
return;
+ clear_header_padding (message);
+
get_string_field (message, field, &len);
/* The field typecode and name take up 8 bytes, and the nul
@@ -332,6 +383,8 @@ delete_string_field (DBusMessage *message,
adjust_field_offsets (message,
offset - 8,
- delete_len);
+
+ append_header_padding (message);
}
static dbus_bool_t
@@ -528,8 +581,8 @@ static dbus_bool_t
dbus_message_create_header (DBusMessage *message,
const char *service,
const char *name)
-{
- if (!_dbus_string_append_byte (&message->header, DBUS_COMPILER_BYTE_ORDER))
+{
+ if (!_dbus_string_append_byte (&message->header, message->byte_order))
return FALSE;
if (!_dbus_string_append_len (&message->header, "\0\0\0", 3))
@@ -563,7 +616,7 @@ dbus_message_create_header (DBusMessage *message,
DBUS_HEADER_FIELD_NAME,
name))
return FALSE;
-
+
return TRUE;
}
@@ -969,7 +1022,7 @@ dbus_message_append_int32 (DBusMessage *message,
}
return _dbus_marshal_int32 (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value);
+ message->byte_order, value);
}
/**
@@ -992,7 +1045,7 @@ dbus_message_append_uint32 (DBusMessage *message,
}
return _dbus_marshal_uint32 (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value);
+ message->byte_order, value);
}
/**
@@ -1015,7 +1068,7 @@ dbus_message_append_double (DBusMessage *message,
}
return _dbus_marshal_double (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value);
+ message->byte_order, value);
}
/**
@@ -1038,7 +1091,7 @@ dbus_message_append_string (DBusMessage *message,
}
return _dbus_marshal_string (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value);
+ message->byte_order, value);
}
/**
@@ -1063,7 +1116,7 @@ dbus_message_append_byte_array (DBusMessage *message,
}
return _dbus_marshal_byte_array (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value, len);
+ message->byte_order, value, len);
}
/**
@@ -1088,7 +1141,7 @@ dbus_message_append_string_array (DBusMessage *message,
}
return _dbus_marshal_string_array (&message->body,
- DBUS_COMPILER_BYTE_ORDER, value, len);
+ message->byte_order, value, len);
}
/**
@@ -1342,9 +1395,9 @@ dbus_message_iter_has_next (DBusMessageIter *iter)
{
int end_pos;
- if (!_dbus_marshal_get_field_end_pos (&iter->message->body,
- iter->message->byte_order,
- iter->pos, &end_pos))
+ if (!_dbus_marshal_get_arg_end_pos (&iter->message->body,
+ iter->message->byte_order,
+ iter->pos, &end_pos))
return FALSE;
if (end_pos >= _dbus_string_get_length (&iter->message->body))
@@ -1364,8 +1417,9 @@ dbus_message_iter_next (DBusMessageIter *iter)
{
int end_pos;
- if (!_dbus_marshal_get_field_end_pos (&iter->message->body, iter->message->byte_order,
- iter->pos, &end_pos))
+ if (!_dbus_marshal_get_arg_end_pos (&iter->message->body,
+ iter->message->byte_order,
+ iter->pos, &end_pos))
return FALSE;
if (end_pos >= _dbus_string_get_length (&iter->message->body))
@@ -1570,6 +1624,12 @@ dbus_message_get_sender (DBusMessage *message)
*
*/
+/* we definitely use signed ints for sizes, so don't exceed
+ * _DBUS_INT_MAX; and add 16 for paranoia, since a message
+ * over 128M is pretty nuts anyhow.
+ */
+#define MAX_SANE_MESSAGE_SIZE (_DBUS_INT_MAX/16)
+
/**
* Implementation details of DBusMessageLoader.
* All members are private.
@@ -1724,18 +1784,11 @@ _dbus_message_loader_get_buffer (DBusMessageLoader *loader,
#define DBUS_HEADER_FIELD_SENDER_AS_UINT32 \
FOUR_CHARS_TO_UINT32 ('s', 'n', 'd', 'r')
-
-/* 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,
- int byte_order,
- HeaderField fields[FIELD_LAST])
+decode_header_data (const DBusString *data,
+ int header_len,
+ int byte_order,
+ HeaderField fields[FIELD_LAST])
{
const char *field;
int pos, new_pos;
@@ -1743,7 +1796,7 @@ decode_header_data (DBusString *data,
if (header_len < 16)
return FALSE;
-
+
i = 0;
while (i < FIELD_LAST)
{
@@ -1755,9 +1808,14 @@ decode_header_data (DBusString *data,
fields[FIELD_BODY_LENGTH].offset = 8;
fields[FIELD_CLIENT_SERIAL].offset = 12;
- /* Now handle the fields */
+ /* Now handle the named fields. A real named field is at least 4
+ * bytes for the name, plus a type code (1 byte) plus padding. So
+ * if we have less than 8 bytes left, it must be alignment padding,
+ * not a field. While >= 8 bytes can't be entirely alignment
+ * padding.
+ */
pos = 16;
- while (pos < header_len)
+ while ((pos + 7) < header_len)
{
pos = _DBUS_ALIGN_VALUE (pos, 4);
@@ -1833,8 +1891,7 @@ decode_header_data (DBusString *data,
field[0], field[1], field[2], field[3]);
}
- /* This function has to validate the fields */
- if (!_dbus_marshal_get_field_end_pos (data, byte_order, pos, &new_pos))
+ if (!_dbus_marshal_validate_arg (data, byte_order, pos, &new_pos))
return FALSE;
if (new_pos > header_len)
@@ -1842,6 +1899,19 @@ decode_header_data (DBusString *data,
pos = new_pos;
}
+
+ if (pos < header_len)
+ {
+ /* Alignment padding, verify that it's nul */
+ _dbus_assert ((header_len - pos) < 8);
+
+ if (!_dbus_string_validate_nul (data,
+ pos, (header_len - pos)))
+ {
+ _dbus_verbose ("header alignment padding is not nul\n");
+ return FALSE;
+ }
+ }
return TRUE;
}
@@ -1893,6 +1963,22 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader,
header_len = _dbus_unpack_int32 (byte_order, header_data + 4);
body_len = _dbus_unpack_int32 (byte_order, header_data + 8);
+ if (header_len < 16 || body_len < 0)
+ {
+ _dbus_verbose ("Message had broken too-small header or body len %d %d\n",
+ header_len, body_len);
+ loader->corrupted = TRUE;
+ return;
+ }
+
+ if (_DBUS_ALIGN_VALUE (header_len, 8) != (unsigned int) header_len)
+ {
+ _dbus_verbose ("header length %d is not aligned to 8 bytes\n",
+ header_len);
+ loader->corrupted = TRUE;
+ return;
+ }
+
if (header_len + body_len > loader->max_message_size)
{
_dbus_verbose ("Message claimed length header = %d body = %d exceeds max message length %d\n",
@@ -1905,22 +1991,47 @@ _dbus_message_loader_return_buffer (DBusMessageLoader *loader,
{
HeaderField fields[FIELD_LAST];
int i;
-
- /* FIXME right now if this doesn't have enough memory, the
- * loader becomes corrupted. Instead we should just not
- * parse this message for now.
- */
+ int next_arg;
+
if (!decode_header_data (&loader->data, header_len, byte_order,
fields))
{
loader->corrupted = TRUE;
return;
}
+
+ next_arg = header_len;
+ while (next_arg < (header_len + body_len))
+ {
+ int prev = next_arg;
+
+ if (!_dbus_marshal_validate_arg (&loader->data,
+ byte_order,
+ next_arg,
+ &next_arg))
+ {
+ loader->corrupted = TRUE;
+ return;
+ }
+
+ _dbus_assert (next_arg > prev);
+ }
+
+ if (next_arg > (header_len + body_len))
+ {
+ _dbus_verbose ("end of last arg at %d but message has len %d+%d=%d\n",
+ next_arg, header_len, body_len,
+ header_len + body_len);
+ loader->corrupted = TRUE;
+ return;
+ }
message = dbus_message_new_empty_header ();
if (message == NULL)
break; /* ugh, postpone this I guess. */
+ message->byte_order = byte_order;
+
/* Copy in the offsets we found */
i = 0;
while (i < FIELD_LAST)
@@ -2012,6 +2123,12 @@ void
_dbus_message_loader_set_max_message_size (DBusMessageLoader *loader,
long size)
{
+ if (size > MAX_SANE_MESSAGE_SIZE)
+ {
+ _dbus_verbose ("clamping requested max message size %ld to %d\n",
+ size, MAX_SANE_MESSAGE_SIZE);
+ size = MAX_SANE_MESSAGE_SIZE;
+ }
loader->max_message_size = size;
}
diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index 9acf5cfb..f453dcb6 100644
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -608,7 +608,7 @@ _dbus_string_align_length (DBusString *str,
int delta;
DBUS_STRING_PREAMBLE (str);
_dbus_assert (alignment >= 1);
- _dbus_assert (alignment <= 16); /* arbitrary */
+ _dbus_assert (alignment <= 8); /* it has to be a bug if > 8 */
new_len = _DBUS_ALIGN_VALUE (real->len, alignment);
@@ -1843,6 +1843,65 @@ _dbus_string_validate_ascii (const DBusString *str,
return TRUE;
}
+/**
+ * Checks that the given range of the string
+ * is valid UTF-8. If the given range is not contained
+ * in the string, returns #FALSE. If the string
+ * contains any nul bytes in the given range, returns
+ * #FALSE.
+ *
+ * @todo right now just calls _dbus_string_validate_ascii()
+ *
+ * @param str the string
+ * @param start first byte index to check
+ * @param len number of bytes to check
+ * @returns #TRUE if the byte range exists and is all valid UTF-8
+ */
+dbus_bool_t
+_dbus_string_validate_utf8 (const DBusString *str,
+ int start,
+ int len)
+{
+ /* FIXME actually validate UTF-8 */
+ return _dbus_string_validate_ascii (str, start, len);
+}
+
+/**
+ * Checks that the given range of the string
+ * is all nul bytes. If the given range is
+ * not contained in the string, returns #FALSE.
+ *
+ * @param str the string
+ * @param start first byte index to check
+ * @param len number of bytes to check
+ * @returns #TRUE if the byte range exists and is all nul bytes
+ */
+dbus_bool_t
+_dbus_string_validate_nul (const DBusString *str,
+ int start,
+ int len)
+{
+ const unsigned char *s;
+ const unsigned char *end;
+ DBUS_CONST_STRING_PREAMBLE (str);
+ _dbus_assert (start >= 0);
+ _dbus_assert (len >= 0);
+
+ if ((start + len) > real->len)
+ return FALSE;
+
+ s = real->str + start;
+ end = s + len;
+ while (s != end)
+ {
+ if (*s != '\0')
+ return FALSE;
+ ++s;
+ }
+
+ return TRUE;
+}
+
/** @} */
#ifdef DBUS_BUILD_TESTS
diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
index c762a68f..e71f7fee 100644
--- a/dbus/dbus-string.h
+++ b/dbus/dbus-string.h
@@ -181,6 +181,14 @@ dbus_bool_t _dbus_string_validate_ascii (const DBusString *str,
int start,
int len);
+dbus_bool_t _dbus_string_validate_utf8 (const DBusString *str,
+ int start,
+ int len);
+
+dbus_bool_t _dbus_string_validate_nul (const DBusString *str,
+ int start,
+ int len);
+
DBUS_END_DECLS;
#endif /* DBUS_STRING_H */