2003-04-08 Havoc Pennington <hp@redhat.com>

	* bus/driver.c (bus_driver_handle_acquire_service): init
	retval/reply before checking name

	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add a
	recursion depth argument

	* dbus/dbus-message.h (struct DBusMessageIter): put some padding
	in the public struct for future extension

	* dbus/dbus-message-builder.c (_dbus_message_data_load): fix
	typo

	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): fix a verbose
	message

	* doc/dbus-specification.sgml: fix typo

8 files changed, 63 insertions, 16 deletions

diff --git a/ChangeLog b/ChangeLog
index ec91b569..a81b2d9e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2003-04-08  Havoc Pennington  <hp@redhat.com>
+
+	* bus/driver.c (bus_driver_handle_acquire_service): init
+	retval/reply before checking name
+
+	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add a
+	recursion depth argument
+
+	* dbus/dbus-message.h (struct DBusMessageIter): put some padding
+	in the public struct for future extension
+
+	* dbus/dbus-message-builder.c (_dbus_message_data_load): fix
+	typo
+
+	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): fix a verbose
+	message
+
+	* doc/dbus-specification.sgml: fix typo
+
 2003-04-08  Alexander Larsson  <alexl@redhat.com>
 
 	Implemented recursive types, named types and new-style iters
diff --git a/bus/driver.c b/bus/driver.c
index ba31f3e7..31b848e9 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -457,7 +457,10 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
     return FALSE;
   
   _dbus_verbose ("Trying to own service %s with flags 0x%x\n", name, flags);
-
+  
+  retval = FALSE;
+  reply = NULL;
+  
   if (*name == ':')
     {
       /* Not allowed; only base services can start with ':' */
@@ -467,9 +470,6 @@ bus_driver_handle_acquire_service (DBusConnection *connection,
       
       goto out;
     }
-  
-  retval = FALSE;
-  reply = NULL;
 
   _dbus_string_init_const (&service_name, name);
   
diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c
index a5cea383..83a3e6f2 100644
--- a/dbus/dbus-marshal.c
+++ b/dbus/dbus-marshal.c
@@ -1184,9 +1184,14 @@ _dbus_marshal_validate_type   (const DBusString *str,
  * returns #TRUE if a valid arg begins at "pos"
  *
  * @todo security: need to audit this function.
+ *
+ * @todo For array types that can't be invalid, we should not
+ * walk the whole array validating it. e.g. just skip all the
+ * int values in an int array.
  * 
  * @param str a string
  * @param byte_order the byte order to use
+ * @param depth current recursion depth, to prevent excessive recursion
  * @param type the type of the argument
  * @param pos the pos where the arg starts
  * @param end_pos pointer where the position right
@@ -1196,13 +1201,25 @@ _dbus_marshal_validate_type   (const DBusString *str,
 dbus_bool_t
 _dbus_marshal_validate_arg (const DBusString *str,
                             int	              byte_order,
+                            int               depth,
 			    int               type,
                             int               pos,
                             int              *end_pos)
 {
   if (pos > _dbus_string_get_length (str))
-    return FALSE;
+    {
+      _dbus_verbose ("Validation went off the end of the message\n");
+      return FALSE;
+    }
 
+#define MAX_VALIDATION_DEPTH 32
+  
+  if (depth > MAX_VALIDATION_DEPTH)
+    {
+      _dbus_verbose ("Maximum recursion depth reached validating message\n");
+      return FALSE;
+    }
+  
   switch (type)
     {
     case DBUS_TYPE_INVALID:
@@ -1216,7 +1233,7 @@ _dbus_marshal_validate_arg (const DBusString *str,
     case DBUS_TYPE_BYTE:
       if (1 > _dbus_string_get_length (str) - pos)
 	{
-	  _dbus_verbose ("no room for boolean value\n");
+	  _dbus_verbose ("no room for byte value\n");
 	  return FALSE;
 	}
 	
@@ -1342,7 +1359,7 @@ _dbus_marshal_validate_arg (const DBusString *str,
 	
 	while (pos < end)
 	  {
-	    if (!_dbus_marshal_validate_arg (str, byte_order,
+	    if (!_dbus_marshal_validate_arg (str, byte_order, depth + 1,
 					     array_type, pos, &pos))
 	      return FALSE;
 	  }
@@ -1378,7 +1395,7 @@ _dbus_marshal_validate_arg (const DBusString *str,
 	while (pos < end)
 	  {
 	    /* Validate name */
-	    if (!_dbus_marshal_validate_arg (str, byte_order,
+	    if (!_dbus_marshal_validate_arg (str, byte_order, depth + 1,
 					     DBUS_TYPE_STRING, pos, &pos))
 	      return FALSE;
 	    
@@ -1389,7 +1406,7 @@ _dbus_marshal_validate_arg (const DBusString *str,
 	      }
 	    
 	    /* Validate element */
-	    if (!_dbus_marshal_validate_arg (str, byte_order,
+	    if (!_dbus_marshal_validate_arg (str, byte_order, depth + 1,
 					     dict_type, pos, &pos))
 	      return FALSE;
 	  }
diff --git a/dbus/dbus-marshal.h b/dbus/dbus-marshal.h
index 0f40cd70..81ff6f50 100644
--- a/dbus/dbus-marshal.h
+++ b/dbus/dbus-marshal.h
@@ -183,6 +183,7 @@ dbus_bool_t _dbus_marshal_validate_type   (const DBusString *str,
 					   int              *type,
                                            int              *end_pos);
 dbus_bool_t _dbus_marshal_validate_arg    (const DBusString *str,
+                                           int               depth,
                                            int               byte_order,
 					   int               type,
                                            int               pos,
diff --git a/dbus/dbus-message-builder.c b/dbus/dbus-message-builder.c
index dbfe3239..93d65e62 100644
--- a/dbus/dbus-message-builder.c
+++ b/dbus/dbus-message-builder.c
@@ -689,7 +689,7 @@ _dbus_message_data_load (DBusString       *dest,
 		  values = dbus_realloc (values, allocated * sizeof (unsigned char));
 		  if (!values)
 		    {
-		      _dbus_warn ("could not allocate memory for BOOLEAN_ARRAY\n");
+		      _dbus_warn ("could not allocate memory for BYTE_ARRAY\n");
 		      goto parse_failed;
 		    }
 		}
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 994e160d..35cf1b5a 100644
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -352,6 +352,10 @@ append_string_field (DBusMessage *message,
   return FALSE;
 }
 
+#ifdef DBUS_BUILD_TESTS
+/* This isn't used, but building it when tests are enabled just to
+ * keep it compiling if we need it in future
+ */
 static void
 delete_int_field (DBusMessage *message,
                   int          field)
@@ -379,6 +383,7 @@ delete_int_field (DBusMessage *message,
 
   append_header_padding (message);
 }
+#endif
 
 static void
 delete_string_field (DBusMessage *message,
@@ -1555,6 +1560,8 @@ dbus_message_iter_init (DBusMessage *message,
 			DBusMessageIter *iter)
 {
   DBusMessageRealIter *real = (DBusMessageRealIter *)iter;
+
+  _dbus_assert (sizeof (DBusMessageRealIter) <= sizeof (DBusMessageIter));
   
   real->message = message;
   real->parent_iter = NULL;
@@ -1771,7 +1778,7 @@ dbus_message_iter_get_string (DBusMessageIter *iter)
 
 /**
  * Returns the name and data from a named type that an
- * iterator may point to.Note that you need to check that
+ * iterator may point to. Note that you need to check that
  * the iterator points to a named type before using this
  * function.
  *
@@ -3520,7 +3527,7 @@ decode_header_data (const DBusString   *data,
 	  return FALSE;
 	}
       
-      if (!_dbus_marshal_validate_arg (data, byte_order, type, pos, &new_pos))
+      if (!_dbus_marshal_validate_arg (data, byte_order, 0, type, pos, &new_pos))
         {
           _dbus_verbose ("Failed to validate argument to named header field\n");
           return FALSE;
@@ -3701,6 +3708,7 @@ _dbus_message_loader_queue_messages (DBusMessageLoader *loader)
       
               if (!_dbus_marshal_validate_arg (&loader->data,
                                                byte_order,
+                                               0,
 					       type,
                                                next_arg,
                                                &next_arg))
@@ -4029,7 +4037,7 @@ check_message_handling_type (DBusMessageIter *iter,
 	str = dbus_message_iter_get_string (iter);
 	if (str == NULL)
 	  {
-	    _dbus_warn ("NULL string int message\n");
+	    _dbus_warn ("NULL string in message\n");
 	    return FALSE;
 	  }
 	dbus_free (str);
@@ -4731,8 +4739,7 @@ _dbus_message_test (const char *test_data_dir)
   const char *name2;
   const dbus_uint32_t our_int32_array[] = { 0x12345678, 0x23456781, 0x34567812, 0x45678123 };
 
-
-  _dbus_assert (sizeof (DBusMessageRealIter) == sizeof (DBusMessageIter));
+  _dbus_assert (sizeof (DBusMessageRealIter) <= sizeof (DBusMessageIter));
 
   /* Test the vararg functions */
   message = dbus_message_new ("org.freedesktop.DBus.Test", "testMessage");
diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h
index 6c82cf07..6a940536 100644
--- a/dbus/dbus-message.h
+++ b/dbus/dbus-message.h
@@ -48,6 +48,9 @@ struct DBusMessageIter
   int dummy8;
   int dummy9;
   int dummy10;
+  int pad1;
+  int pad2;
+  void *pad3;
 };
 
 
diff --git a/doc/dbus-specification.sgml b/doc/dbus-specification.sgml
index 200ef430..5a7a80fa 100644
--- a/doc/dbus-specification.sgml
+++ b/doc/dbus-specification.sgml
@@ -396,7 +396,7 @@
                 <entry>a byte giving the element type of the array followed
                 by an UINT32 (aligned to 4 bytes) giving the length of the
                 array data in bytes. This is then followed by a number of
-                entires with the same type, encoded like that type normally
+                entries with the same type, encoded like that type normally
                 would be encoded alone.
                 </entry>
               </row><row>