Commit cfdfcac5 authored by Simon McVittie's avatar Simon McVittie

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's avatarSimon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350Reviewed-by: Thiago Macieira's avatarThiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
(cherry picked from commit f4296313)
parent 4c596ecc
......@@ -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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment