diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | bus/selinux.c | 15 | ||||
| -rw-r--r-- | dbus/dbus-string.c | 34 | ||||
| -rw-r--r-- | dbus/dbus-string.h | 3 |
4 files changed, 58 insertions, 8 deletions
@@ -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 735a76d4..d31f9386 100644 --- 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 000b4f64..cb108a8d 100644 --- 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 b0100f3a..d88d67ed 100644 --- 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 */ |