From 218f7aee108381cd076062b161eea02af7ab8903 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 23 Jul 2018 18:52:01 +0100 Subject: [PATCH] sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un Using strncpy (buffer, str, strlen (str)) is a "code smell" that might indicate a serious bug (it effectively turns strncpy into strcpy), and gcc 8 now warns about it. In fact we avoided the bug here, but it wasn't at all obvious. We already checked that path_len is less than or equal to _DBUS_MAX_SUN_PATH_LENGTH, which is 99, chosen to be strictly less than the POSIX minimum sizeof(sun_path) >= 100, so we couldn't actually be overflowing the available buffer. The new static assertion in this commit matches a comment above the definition of _DBUS_MAX_SUN_PATH_LENGTH: we define _DBUS_MAX_SUN_PATH_LENGTH to 99, because POSIX says struct sockaddr_un's sun_path member is at least 100 bytes (including space for a \0 terminator). dbus will now fail to compile on platforms that are non-POSIX-compliant in this way, except for Windows. We zeroed the struct sockaddr_un before writing into it, so stopping one byte short of the end of sun_path ensures that we get \0 termination. Signed-off-by: Simon McVittie --- dbus/dbus-sysdeps-unix.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index b2b30f19..b7309712 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -910,6 +910,7 @@ _dbus_connect_unix_socket (const char *path, int fd; size_t path_len; struct sockaddr_un addr; + _DBUS_STATIC_ASSERT (sizeof (addr.sun_path) > _DBUS_MAX_SUN_PATH_LENGTH); _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -942,7 +943,7 @@ _dbus_connect_unix_socket (const char *path, return -1; } - strncpy (&addr.sun_path[1], path, path_len); + strncpy (&addr.sun_path[1], path, sizeof (addr.sun_path) - 2); /* _dbus_verbose_bytes (addr.sun_path, sizeof (addr.sun_path)); */ #else /* HAVE_ABSTRACT_SOCKETS */ dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, @@ -961,7 +962,7 @@ _dbus_connect_unix_socket (const char *path, return -1; } - strncpy (addr.sun_path, path, path_len); + strncpy (addr.sun_path, path, sizeof (addr.sun_path) - 1); } if (connect (fd, (struct sockaddr*) &addr, _DBUS_STRUCT_OFFSET (struct sockaddr_un, sun_path) + path_len) < 0) @@ -1112,6 +1113,7 @@ _dbus_listen_unix_socket (const char *path, int listen_fd; struct sockaddr_un addr; size_t path_len; + _DBUS_STATIC_ASSERT (sizeof (addr.sun_path) > _DBUS_MAX_SUN_PATH_LENGTH); _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -1146,7 +1148,7 @@ _dbus_listen_unix_socket (const char *path, return -1; } - strncpy (&addr.sun_path[1], path, path_len); + strncpy (&addr.sun_path[1], path, sizeof (addr.sun_path) - 2); /* _dbus_verbose_bytes (addr.sun_path, sizeof (addr.sun_path)); */ #else /* HAVE_ABSTRACT_SOCKETS */ dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, @@ -1183,7 +1185,7 @@ _dbus_listen_unix_socket (const char *path, return -1; } - strncpy (addr.sun_path, path, path_len); + strncpy (addr.sun_path, path, sizeof (addr.sun_path) - 1); } if (bind (listen_fd, (struct sockaddr*) &addr, _DBUS_STRUCT_OFFSET (struct sockaddr_un, sun_path) + path_len) < 0) -- 2.18.0