2003-03-02 Havoc Pennington <hp@pobox.com>

	* test/break-loader.c (randomly_set_extreme_ints): add test that
	sets really huge and small integers

	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add check
	that length of boolean array fits in the string, and that
	string has room for boolean value in single-bool case.

	* dbus/dbus-message-builder.c (_dbus_message_data_load): add
	optional value to "ALIGN" command which is what to fill the
	alignment with.

	* test/data/valid-messages/no-padding.message: add regression
	test for the message padding problem

8 files changed, 167 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 1505e69f..97b81bdf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2003-03-02  Havoc Pennington  <hp@pobox.com>
 
+	* test/break-loader.c (randomly_set_extreme_ints): add test that
+	sets really huge and small integers
+
+	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): add check
+	that length of boolean array fits in the string, and that 
+	string has room for boolean value in single-bool case.
+
+	* dbus/dbus-message-builder.c (_dbus_message_data_load): add
+	optional value to "ALIGN" command which is what to fill the
+	alignment with.
+
+	* test/data/valid-messages/no-padding.message: add regression
+	test for the message padding problem
+
+2003-03-02  Havoc Pennington  <hp@pobox.com>
+
 	* dbus/dbus-message.c (decode_header_data): fix to always init
 	message_padding, from Benjamin Dauvergne
 
diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index d928a5c8..19a5cdc3 100644
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -115,6 +115,7 @@ char* _dbus_strdup (const char *str);
 
 #define _DBUS_INT_MIN	(-_DBUS_INT_MAX - 1)
 #define _DBUS_INT_MAX	2147483647
+#define _DBUS_UINT_MAX	0xffffffff
 #define _DBUS_MAX_SUN_PATH_LENGTH 99
 #define _DBUS_ONE_KILOBYTE 1024
 #define _DBUS_ONE_MEGABYTE 1024 * _DBUS_ONE_KILOBYTE
diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c
index b199561b..f78757fd 100644
--- a/dbus/dbus-marshal.c
+++ b/dbus/dbus-marshal.c
@@ -997,7 +997,9 @@ _dbus_marshal_get_arg_end_pos (const DBusString *str,
  * 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.
+ * signed integer. DOES NOT check whether the length points
+ * beyond the end of the string, because it doesn't know the
+ * size of array elements.
  *
  * @param str the string
  * @param byte_order the byte order
@@ -1012,6 +1014,8 @@ demarshal_and_validate_len (const DBusString *str,
 {
   int align_4 = _DBUS_ALIGN_VALUE (pos, 4);
   unsigned int len;
+
+  _dbus_assert (new_pos != NULL);
   
   if ((align_4 + 4) >= _dbus_string_get_length (str))
     {
@@ -1116,6 +1120,12 @@ _dbus_marshal_validate_arg (const DBusString *str,
       {
 	unsigned char c;
 
+        if (2 > _dbus_string_get_length (str) - pos)
+          {
+            _dbus_verbose ("no room for boolean value\n");
+            return FALSE;
+          }
+        
 	c = _dbus_string_get_byte (str, pos + 1);
 
 	if (c != 0 && c != 1)
@@ -1184,6 +1194,12 @@ _dbus_marshal_validate_arg (const DBusString *str,
         if (len < 0)
           return FALSE;
 
+        if (len > _dbus_string_get_length (str) - pos)
+          {
+            _dbus_verbose ("boolean array length outside length of the message\n");
+            return FALSE;
+          }
+        
 	i = 0;
 	while (i < len)
 	  {
diff --git a/dbus/dbus-message-builder.c b/dbus/dbus-message-builder.c
index dea50d7f..3501da9a 100644
--- a/dbus/dbus-message-builder.c
+++ b/dbus/dbus-message-builder.c
@@ -334,6 +334,12 @@ _dbus_message_data_load (DBusString       *dest,
       _dbus_string_free (&file);
       return FALSE;
     }
+
+  {
+    const char *s;
+    _dbus_string_get_const_data (filename, &s);
+    _dbus_verbose ("Loading %s\n", s);
+  }
   
   if ((result = _dbus_file_get_contents (&file, filename)) != DBUS_RESULT_SUCCESS)
     {
@@ -439,24 +445,47 @@ _dbus_message_data_load (DBusString       *dest,
                                                "ALIGN"))
         {
           long val;
-
+          int end;
+          int orig_len;
+          
           _dbus_string_delete_first_word (&line);
 
-          if (!_dbus_string_parse_int (&line, 0, &val, NULL))
+          if (!_dbus_string_parse_int (&line, 0, &val, &end))
             {
               _dbus_warn ("Failed to parse integer\n");
               goto parse_failed;
             }
 
-          if (val > 16)
+          if (val > 8)
             {
               _dbus_warn ("Aligning to %ld boundary is crack\n",
                           val);
               goto parse_failed;
             }
+
+          orig_len = _dbus_string_get_length (dest);
           
           if (!_dbus_string_align_length (dest, val))
             goto parse_failed;
+
+          if (_dbus_string_parse_int (&line, end, &val, NULL))
+            {
+              /* If there's an optional second int argument,
+               * fill in align padding with that value
+               */
+              if (val < 0 || val > 255)
+                {
+                  _dbus_warn ("can't fill align padding with %ld, must be a byte value\n", val);
+                  goto parse_failed;
+                }
+
+              end = orig_len;
+              while (end < _dbus_string_get_length (dest))
+                {
+                  _dbus_string_set_byte (dest, end, val);
+                  ++end;
+                }
+            }
         }
       else if (_dbus_string_starts_with_c_str (&line, "UNALIGN"))
         {
diff --git a/test/break-loader.c b/test/break-loader.c
index db9a14a1..f08f43d9 100644
--- a/test/break-loader.c
+++ b/test/break-loader.c
@@ -335,6 +335,67 @@ randomly_modify_length (const DBusString *orig_data,
 }
 
 static void
+randomly_set_extreme_ints (const DBusString *orig_data,
+                           DBusString       *mutated)
+{
+  int i;
+  int byte_order;
+  const char *d;
+  dbus_uint32_t orig;
+  static int which = 0;
+  unsigned int extreme_ints[] = {
+    _DBUS_INT_MAX,
+    _DBUS_UINT_MAX,
+    _DBUS_INT_MAX - 1,
+    _DBUS_UINT_MAX - 1,
+    _DBUS_INT_MAX - 2,
+    _DBUS_UINT_MAX - 2,
+    (unsigned int) (_DBUS_INT_MAX + 1),
+    (unsigned int) (_DBUS_UINT_MAX + 1),
+    _DBUS_INT_MAX + 2,
+    _DBUS_UINT_MAX + 2,
+    0, 1, 2, 3,
+    (unsigned int) -1,
+    (unsigned int) -2,
+    (unsigned int) -3
+  };
+    
+  if (orig_data != mutated)
+    {
+      _dbus_string_set_length (mutated, 0);
+      
+      if (!_dbus_string_copy (orig_data, 0, mutated, 0))
+        _dbus_assert_not_reached ("out of mem");
+    }
+
+  if (_dbus_string_get_length (mutated) < 12)
+    return;
+  
+  _dbus_string_get_const_data (mutated, &d);
+
+  if (!(*d == DBUS_LITTLE_ENDIAN ||
+        *d == DBUS_BIG_ENDIAN))
+    return;
+
+  byte_order = *d;
+  
+  i = random_int_in_range (4, _dbus_string_get_length (mutated) - 8);
+  i = _DBUS_ALIGN_VALUE (i, 4);
+
+  orig = _dbus_demarshal_uint32 (mutated, byte_order, i, NULL);
+
+  which = random_int_in_range (0, _DBUS_N_ELEMENTS (extreme_ints));
+
+  _dbus_assert (which >= 0);
+  _dbus_assert (which < _DBUS_N_ELEMENTS (extreme_ints));
+  
+  _dbus_marshal_set_uint32 (mutated, byte_order, i,
+                            extreme_ints[which]);
+}
+
+static int times_we_did_each_thing[6] = { 0, };
+
+static void
 randomly_do_n_things (const DBusString *orig_data,
                       DBusString       *mutated,
                       int               n)
@@ -347,7 +408,8 @@ randomly_do_n_things (const DBusString *orig_data,
       randomly_change_one_byte,
       randomly_add_one_byte,
       randomly_remove_one_byte,
-      randomly_modify_length
+      randomly_modify_length,
+      randomly_set_extreme_ints
     };
 
   _dbus_string_set_length (mutated, 0);
@@ -363,6 +425,7 @@ randomly_do_n_things (const DBusString *orig_data,
       which = random_int_in_range (0, _DBUS_N_ELEMENTS (functions));
 
       (* functions[which]) (mutated, mutated);
+      times_we_did_each_thing[which] += 1;
       
       ++i;
     }
@@ -432,6 +495,15 @@ find_breaks_based_on (const DBusString   *filename,
 
       ++i;
     }
+
+  i = 0;
+  while (i < 50)
+    {
+      randomly_set_extreme_ints (&orig_data, &mutated);
+      try_mutated_data (&mutated);
+
+      ++i;
+    }
   
   i = 0;
   while (i < 15)
@@ -588,6 +660,15 @@ main (int    argc,
           return 1;
         }
 
+      printf ("  did %d random mutations: %d %d %d %d %d %d\n",
+              _DBUS_N_ELEMENTS (times_we_did_each_thing),
+              times_we_did_each_thing[0],
+              times_we_did_each_thing[1],
+              times_we_did_each_thing[2],
+              times_we_did_each_thing[3],
+              times_we_did_each_thing[4],
+              times_we_did_each_thing[5]);
+      
       printf ("Found %d failures with seed %u stored in %s\n",
               failures_this_iteration, seed, failure_dir_c);
 
diff --git a/test/data/invalid-messages/boolean-array-length-too-long.message-raw b/test/data/invalid-messages/boolean-array-length-too-long.message-raw
new file mode 100644
index 00000000..2326ec9d
--- /dev/null
+++ b/test/data/invalid-messages/boolean-array-length-too-long.message-raw
diff --git a/test/data/invalid-messages/boolean-has-no-value.message-raw b/test/data/invalid-messages/boolean-has-no-value.message-raw
new file mode 100644
index 00000000..cba9e839
--- /dev/null
+++ b/test/data/invalid-messages/boolean-has-no-value.message-raw
diff --git a/test/data/valid-messages/no-padding.message b/test/data/valid-messages/no-padding.message
new file mode 100644
index 00000000..c21c84d3
--- /dev/null
+++ b/test/data/valid-messages/no-padding.message
@@ -0,0 +1,19 @@
+## Message with no header padding
+
+## VALID_HEADER includes a LENGTH Header and LENGTH Body
+VALID_HEADER
+
+## this byte array is filled with zeros to the natural length 
+## of the header
+FIELD_NAME unkn
+TYPE BYTE_ARRAY
+ALIGN 4
+LENGTH ThisByteArray
+START_LENGTH ThisByteArray
+BYTE 1
+ALIGN 8 1
+END_LENGTH ThisByteArray
+
+END_LENGTH Header
+START_LENGTH Body
+END_LENGTH Body