2005-02-05 Havoc Pennington <hp@redhat.com>

	* dbus/dbus-marshal-validate.c (validate_body_helper): fix crash
	if the signature of a variant was empty
	(_dbus_validate_signature_with_reason): catch "(a)" (array inside
	struct with no element type)

	* dbus/dbus-message-factory.c (generate_uint32_changed): add more
	mangled messages to break things

5 files changed, 190 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index dca9b596..1d35dd0c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2005-02-05  Havoc Pennington  <hp@redhat.com>
+
+	* dbus/dbus-marshal-validate.c (validate_body_helper): fix crash
+	if the signature of a variant was empty
+	(_dbus_validate_signature_with_reason): catch "(a)" (array inside
+	struct with no element type)
+
+	* dbus/dbus-message-factory.c (generate_uint32_changed): add more
+	mangled messages to break things
+
 2005-02-04  Havoc Pennington  <hp@redhat.com>
 
 	* glib/dbus-gproxy.c (dbus_g_proxy_disconnect_signal): use
diff --git a/dbus/dbus-marshal-validate-util.c b/dbus/dbus-marshal-validate-util.c
index 7a1648fa..afaf5262 100644
--- a/dbus/dbus-marshal-validate-util.c
+++ b/dbus/dbus-marshal-validate-util.c
@@ -89,7 +89,13 @@ static const ValidityTest signature_tests[] = {
   { "(())", DBUS_INVALID_STRUCT_HAS_NO_FIELDS },
   { "a()", DBUS_INVALID_STRUCT_HAS_NO_FIELDS },
   { "i()", DBUS_INVALID_STRUCT_HAS_NO_FIELDS },
-  { "()i", DBUS_INVALID_STRUCT_HAS_NO_FIELDS }
+  { "()i", DBUS_INVALID_STRUCT_HAS_NO_FIELDS },
+  { "(a)", DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE },
+  { "a{ia}", DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE },
+  { "a{}", DBUS_INVALID_DICT_ENTRY_HAS_NO_FIELDS }
+  /* { "a{i}", DBUS_INVALID_DICT_ENTRY_HAS_ONLY_ONE_FIELD }, */
+  /* { "{is}", DBUS_INVALID_DICT_ENTRY_NOT_INSIDE_ARRAY }, */
+  /* { "a{isi}", DBUS_INVALID_DICT_ENTRY_HAS_TOO_MANY_FIELDS }, */
 };
 
 dbus_bool_t
diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
index b144b62a..f7b46c0b 100644
--- a/dbus/dbus-marshal-validate.c
+++ b/dbus/dbus-marshal-validate.c
@@ -144,8 +144,16 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
           return DBUS_INVALID_UNKNOWN_TYPECODE;
         }
 
-      if (*p != DBUS_TYPE_ARRAY)
-        array_depth = 0;
+      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;
+          else
+            array_depth = 0;
+        }
 
       last = *p;
       ++p;
@@ -159,6 +167,10 @@ _dbus_validate_signature_with_reason (const DBusString *type_str,
 
   if (dict_entry_depth > 0)
     return DBUS_INVALID_DICT_ENTRY_STARTED_BUT_NOT_ENDED;
+
+  _dbus_assert (last != DBUS_TYPE_ARRAY);
+  _dbus_assert (last != DBUS_STRUCT_BEGIN_CHAR);
+  _dbus_assert (last != DBUS_DICT_ENTRY_BEGIN_CHAR);
   
   return DBUS_VALID;
 }
@@ -362,6 +374,7 @@ validate_body_helper (DBusTypeReader       *reader,
             DBusTypeReader sub;
             DBusValidity validity;
             int contained_alignment;
+            int contained_type;
 
             claimed_len = *p;
             ++p;
@@ -381,7 +394,11 @@ validate_body_helper (DBusTypeReader       *reader,
               return DBUS_INVALID_VARIANT_SIGNATURE_MISSING_NUL;
             ++p;
 
-            contained_alignment = _dbus_type_get_alignment (_dbus_first_type_in_signature (&sig, 0));
+            contained_type = _dbus_first_type_in_signature (&sig, 0);
+            if (contained_type == DBUS_TYPE_INVALID)
+              return DBUS_INVALID_VARIANT_SIGNATURE_EMPTY;
+            
+            contained_alignment = _dbus_type_get_alignment (contained_type);
             
             a = _DBUS_ALIGN_ADDRESS (p, contained_alignment);
             if (a > end)
@@ -395,8 +412,7 @@ validate_body_helper (DBusTypeReader       *reader,
 
             _dbus_type_reader_init_types_only (&sub, &sig, 0);
 
-            if (_dbus_type_reader_get_current_type (&sub) == DBUS_TYPE_INVALID)
-              return DBUS_INVALID_VARIANT_SIGNATURE_EMPTY;
+            _dbus_assert (_dbus_type_reader_get_current_type (&sub) != DBUS_TYPE_INVALID);
 
             validity = validate_body_helper (&sub, byte_order, FALSE, p, end, &p);
             if (validity != DBUS_VALID)
diff --git a/dbus/dbus-message-factory.c b/dbus/dbus-message-factory.c
index b1b90d47..26109815 100644
--- a/dbus/dbus-message-factory.c
+++ b/dbus/dbus-message-factory.c
@@ -28,6 +28,12 @@
 #include "dbus-test.h"
 #include <stdio.h>
 
+typedef enum
+  {
+    CHANGE_TYPE_ADJUST,
+    CHANGE_TYPE_ABSOLUTE
+  } ChangeType;
+
 #define BYTE_ORDER_OFFSET  0
 #define BODY_LENGTH_OFFSET 4
 
@@ -36,11 +42,13 @@ iter_recurse (DBusMessageDataIter *iter)
 {
   iter->depth += 1;
   _dbus_assert (iter->depth < _DBUS_MESSAGE_DATA_MAX_NESTING);
+  _dbus_assert (iter->sequence_nos[iter->depth] >= 0);
 }
 
 static int
 iter_get_sequence (DBusMessageDataIter *iter)
 {
+  _dbus_assert (iter->sequence_nos[iter->depth] >= 0);
   return iter->sequence_nos[iter->depth];
 }
 
@@ -48,6 +56,7 @@ static void
 iter_set_sequence (DBusMessageDataIter *iter,
                    int                  sequence)
 {
+  _dbus_assert (sequence >= 0);
   iter->sequence_nos[iter->depth] = sequence;
 }
 
@@ -354,7 +363,7 @@ generate_byte_changed (DBusMessageDataIter *iter,
   byte_seq = iter_get_sequence (iter);
   iter_next (iter);
   iter_unrecurse (iter);
-
+  
   if (byte_seq == _dbus_string_get_length (data))
     {
       _dbus_string_set_length (data, 0);
@@ -381,6 +390,137 @@ generate_byte_changed (DBusMessageDataIter *iter,
 
 typedef struct
 {
+  ChangeType type;
+  dbus_uint32_t value; /* cast to signed for adjusts */
+} UIntChange;
+
+static const UIntChange uint32_changes[] = {
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -1 },
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -2 },
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) -3 },
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 1 },
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 2 },
+  { CHANGE_TYPE_ADJUST, (dbus_uint32_t) 3 },
+  { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX },
+  { CHANGE_TYPE_ABSOLUTE, 0 },
+  { CHANGE_TYPE_ABSOLUTE, 1 },
+  { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX - 1 },
+  { CHANGE_TYPE_ABSOLUTE, _DBUS_UINT32_MAX - 5 }
+};
+
+static dbus_bool_t
+generate_uint32_changed (DBusMessageDataIter *iter,
+                         DBusString          *data,
+                         DBusValidity        *expected_validity)
+{
+  int body_seq;
+  int byte_seq;
+  int change_seq;
+  dbus_uint32_t v_UINT32;
+  int byte_order;
+  const UIntChange *change;
+  int base_depth;
+
+  /* Outer loop is each body, next loop is each change,
+   * inner loop is each change location
+   */
+
+  base_depth = iter->depth;
+  
+ next_body:
+  _dbus_assert (iter->depth == (base_depth + 0));
+  _dbus_string_set_length (data, 0);
+  body_seq = iter_get_sequence (iter);
+  
+  if (!generate_many_bodies (iter, data, expected_validity))
+    return FALSE;
+
+  _dbus_assert (iter->depth == (base_depth + 0));
+
+  iter_set_sequence (iter, body_seq); /* undo the "next" from generate_many_bodies */
+  iter_recurse (iter);
+ next_change:
+  _dbus_assert (iter->depth == (base_depth + 1));
+  change_seq = iter_get_sequence (iter);
+  
+  if (change_seq == _DBUS_N_ELEMENTS (uint32_changes))
+    {
+      /* Reset change count */
+      iter_set_sequence (iter, 0);
+      iter_unrecurse (iter);
+      iter_next (iter);
+      goto next_body;
+    }
+
+  _dbus_assert (iter->depth == (base_depth + 1));
+  
+  iter_recurse (iter);
+  _dbus_assert (iter->depth == (base_depth + 2));
+  byte_seq = iter_get_sequence (iter);
+  /* skip 4 bytes at a time */
+  iter_next (iter);
+  iter_next (iter);
+  iter_next (iter);
+  iter_next (iter);
+  iter_unrecurse (iter);
+
+  _dbus_assert (_DBUS_ALIGN_VALUE (byte_seq, 4) == (unsigned) byte_seq);
+  if (byte_seq >= (_dbus_string_get_length (data) - 4))
+    {
+      /* reset byte count */
+      _dbus_assert (iter->depth == (base_depth + 1));
+      iter_recurse (iter);
+      _dbus_assert (iter->depth == (base_depth + 2));
+      iter_set_sequence (iter, 0);
+      iter_unrecurse (iter);
+      _dbus_assert (iter->depth == (base_depth + 1));
+      iter_next (iter);
+      goto next_change;
+    }
+  
+  _dbus_assert (byte_seq <= (_dbus_string_get_length (data) - 4));
+
+  byte_order = _dbus_string_get_byte (data, BYTE_ORDER_OFFSET);
+  
+  v_UINT32 = _dbus_marshal_read_uint32 (data, byte_seq, byte_order, NULL);
+
+  change = &uint32_changes[change_seq];
+
+  if (change->type == CHANGE_TYPE_ADJUST)
+    {
+      v_UINT32 += (int) change->value;
+    }
+  else
+    {
+      v_UINT32 = change->value;
+    }
+
+#if 0
+  printf ("body %d change %d pos %d ",
+          body_seq, change_seq, byte_seq);
+
+  if (change->type == CHANGE_TYPE_ADJUST)
+    printf ("adjust by %d", (int) change->value);
+  else
+    printf ("set to %u", change->value);
+  
+  printf (" \t%u -> %u\n",
+          _dbus_marshal_read_uint32 (data, byte_seq, byte_order, NULL),
+          v_UINT32);
+#endif
+  
+  _dbus_marshal_set_uint32 (data, byte_seq, v_UINT32, byte_order);
+  *expected_validity = DBUS_VALIDITY_UNKNOWN;
+
+  _dbus_assert (iter->depth == (base_depth + 1));
+  iter_unrecurse (iter);
+  _dbus_assert (iter->depth == (base_depth + 0));
+          
+  return TRUE;
+}
+
+typedef struct
+{
   const char *name;
   DBusMessageGeneratorFunc func;  
 } DBusMessageGenerator;
@@ -388,6 +528,7 @@ typedef struct
 static const DBusMessageGenerator generators[] = {
   { "trivial example of each message type", generate_trivial },
   { "assorted arguments", generate_many_bodies },
+  { "each uint32 modified", generate_uint32_changed },
   { "wrong body lengths", generate_wrong_length },
   { "each byte modified", generate_byte_changed }
 };
@@ -409,7 +550,8 @@ _dbus_message_data_iter_init (DBusMessageDataIter *iter)
     {
       iter->sequence_nos[i] = 0;
       ++i;
-    } 
+    }
+  iter->count = 0;
 }
 
 dbus_bool_t
@@ -428,7 +570,10 @@ _dbus_message_data_iter_get_and_next (DBusMessageDataIter *iter,
   iter_recurse (iter);
   
   if (iter_first_in_series (iter))
-    printf (" testing message loading: %s\n", generators[generator].name);
+    {
+      printf (" testing message loading: %s ", generators[generator].name);
+      fflush (stdout);
+    }
   
   func = generators[generator].func;
 
@@ -443,10 +588,12 @@ _dbus_message_data_iter_get_and_next (DBusMessageDataIter *iter,
       iter_unrecurse (iter);
       iter_next (iter); /* next generator */
       _dbus_string_free (&data->data);
+      printf ("%d test loads cumulative\n", iter->count);
       goto restart;
     }
   iter_unrecurse (iter);
-  
+
+  iter->count += 1;
   return TRUE;
 }
 
diff --git a/dbus/dbus-message-factory.h b/dbus/dbus-message-factory.h
index 8d992a4f..fe52aedb 100644
--- a/dbus/dbus-message-factory.h
+++ b/dbus/dbus-message-factory.h
@@ -45,6 +45,7 @@ typedef struct
 {
   int sequence_nos[_DBUS_MESSAGE_DATA_MAX_NESTING];
   int depth;
+  int count;
 } DBusMessageDataIter;
 
 void        _dbus_message_data_free              (DBusMessageData     *data);