diff options
author | Simon McVittie <smcv@collabora.com> | 2018-07-23 18:52:01 +0100 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2018-08-02 17:13:02 +0100 |
commit | 4937a36c310c94644a60bc9fce8bde5f511395f5 (patch) | |
tree | 071d0fe97802b26769d6f1ec6b1afa34699e4bd5 /dbus/dbus-sysdeps-unix.c | |
parent | ab8ef4ae31bd5aa28dd228ceb8794877ccf9fd32 (diff) | |
download | dbus-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)
Diffstat (limited to 'dbus/dbus-sysdeps-unix.c')
-rw-r--r-- | dbus/dbus-sysdeps-unix.c | 10 |
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) |