Fix a problem where a nul byte was wrongly introduced into UUIDs, due to _dbus_string...

2007-10-31  Havoc Pennington  <hp@redhat.com>

	* bus/selinux.c (log_audit_callback): rewrite to use
	_dbus_string_copy_to_buffer_with_nul()

	* dbus/dbus-string.c (_dbus_string_copy_to_buffer): change to NOT
	nul-terminate the buffer; fail an assertion if there is not enough
	space in the target buffer. This fixes two bugs where
	copy_to_buffer was used to copy the binary bytes in a UUID, where
	nul termination did not make sense. Bug reported by David Castelow.
	(_dbus_string_copy_to_buffer_with_nul): new function that always
	nul-terminates the buffer, and fails an assertion if there is not
	enough space in the buffer.

diff --git a/ChangeLog b/ChangeLog
index 8e95058..0dc40d8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2007-10-31  Havoc Pennington  <hp@redhat.com>
+
+       * bus/selinux.c (log_audit_callback): rewrite to use
+       _dbus_string_copy_to_buffer_with_nul()
+
+       * dbus/dbus-string.c (_dbus_string_copy_to_buffer): change to NOT
+       nul-terminate the buffer; fail an assertion if there is not enough
+       space in the target buffer. This fixes two bugs where
+       copy_to_buffer was used to copy the binary bytes in a UUID, where
+       nul termination did not make sense. Bug reported by David Castelow.
+       (_dbus_string_copy_to_buffer_with_nul): new function that always
+       nul-terminates the buffer, and fails an assertion if there is not 
+       enough space in the buffer.
+
 2007-10-23  Havoc Pennington  <hp@redhat.com>
 
        * bus/bus.c (bus_context_new): use the new name here

diff --git a/bus/selinux.c b/bus/selinux.c
index 735a76d..d31f938 100644 (file)
--- a/bus/selinux.c
+++ b/bus/selinux.c
@@ -178,7 +178,20 @@ static void
 log_audit_callback (void *data, security_class_t class, char *buf, size_t bufleft)
 {
   DBusString *audmsg = data;
-  _dbus_string_copy_to_buffer (audmsg, buf, bufleft);
+
+  if (bufleft > (size_t) _dbus_string_get_length(audmsg))
+    {
+      _dbus_string_copy_to_buffer_with_nul (audmsg, buf, bufleft);
+    }
+  else
+    {
+      DBusString s;
+
+      _dbus_string_init_const(&s, "Buffer too small for audit message");
+
+      if (bufleft > (size_t) _dbus_string_get_length(&s))
+        _dbus_string_copy_to_buffer_with_nul (&s, buf, bufleft);
+    }
 }
 
 /**

diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index 000b4f6..cb108a8 100644 (file)
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -741,8 +741,9 @@ _dbus_string_copy_data (const DBusString  *str,
 }
 
 /**
- * Copies the contents of a DBusString into a different
- * buffer. The resulting buffer will be nul-terminated.
+ * Copies the contents of a DBusString into a different buffer. It is
+ * a bug if avail_len is too short to hold the string contents. nul
+ * termination is not copied, just the supplied bytes.
  * 
  * @param str a string
  * @param buffer a C buffer to copy data to
@@ -753,15 +754,34 @@ _dbus_string_copy_to_buffer (const DBusString  *str,
                             char              *buffer,
                             int                avail_len)
 {
-  int copy_len;
   DBUS_CONST_STRING_PREAMBLE (str);
 
   _dbus_assert (avail_len >= 0);
+  _dbus_assert (avail_len >= real->len);
+  
+  memcpy (buffer, real->str, real->len);
+}
+
+/**
+ * Copies the contents of a DBusString into a different buffer. It is
+ * a bug if avail_len is too short to hold the string contents plus a
+ * nul byte. 
+ * 
+ * @param str a string
+ * @param buffer a C buffer to copy data to
+ * @param avail_len maximum length of C buffer
+ */
+void
+_dbus_string_copy_to_buffer_with_nul (const DBusString  *str,
+                                      char              *buffer,
+                                      int                avail_len)
+{
+  DBUS_CONST_STRING_PREAMBLE (str);
 
-  copy_len = MIN (avail_len, real->len+1);
-  memcpy (buffer, real->str, copy_len);
-  if (avail_len > 0 && avail_len == copy_len)
-    buffer[avail_len-1] = '\0';
+  _dbus_assert (avail_len >= 0);
+  _dbus_assert (avail_len > real->len);
+  
+  memcpy (buffer, real->str, real->len+1);
 }
 
 #ifdef DBUS_BUILD_TESTS

diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
index b0100f3..d88d67e 100644 (file)
--- a/dbus/dbus-string.h
+++ b/dbus/dbus-string.h
@@ -120,6 +120,9 @@ dbus_bool_t   _dbus_string_copy_data_len         (const DBusString  *str,
 void          _dbus_string_copy_to_buffer        (const DBusString  *str,
                                                   char              *buffer,
                                                  int                len);
+void          _dbus_string_copy_to_buffer_with_nul (const DBusString  *str,
+                                                    char              *buffer,
+                                                    int                avail_len);
 #ifndef _dbus_string_get_length
 int           _dbus_string_get_length            (const DBusString  *str);
 #endif /* !_dbus_string_get_length */