summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSimon McVittie <smcv@collabora.com>2018-07-23 18:52:01 +0100
committerSimon McVittie <smcv@collabora.com>2018-08-02 17:13:02 +0100
commit4937a36c310c94644a60bc9fce8bde5f511395f5 (patch)
tree071d0fe97802b26769d6f1ec6b1afa34699e4bd5
parentab8ef4ae31bd5aa28dd228ceb8794877ccf9fd32 (diff)
downloaddbus-4937a36c310c94644a60bc9fce8bde5f511395f5.tar.gz
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 <smcv@collabora.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350 Reviewed-by: Thiago Macieira <thiago@kde.org> Reviewed-by: Philip Withnall <withnall@endlessm.com> (cherry picked from commit f429631365ba59a1749438af2184cab138a31772)
-rw-r--r--dbus/dbus-sysdeps-unix.c10
1 files changed, 6 insertions, 4 deletions
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 222c8c84..565e089c 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -913,6 +913,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);
@@ -945,7 +946,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 /* !__linux__ */
dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
@@ -964,7 +965,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)
@@ -1115,6 +1116,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);
@@ -1149,7 +1151,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 /* !__linux__ */
dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
@@ -1186,7 +1188,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)