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.

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 */