Created attachment 140793 [details] [review] 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. --- Part 1703 in the ongoing series "why string abstractions are better than strncpy()". sizeof(sun_path) is actually 108 on Linux, and probably other platforms.
D-Bus addresses have to come from a trusted source (if an attacker can induce you to connect to a crafted D-Bus address then you have bigger things to worry about, especially the unixexec: transport) so this wouldn't be a security problem even if it did overflow.
Comment on attachment 140793 [details] [review] sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un Review of attachment 140793 [details] [review]: ----------------------------------------------------------------- LGTM.
Comment on attachment 140793 [details] [review] sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un Review of attachment 140793 [details] [review]: ----------------------------------------------------------------- LGTM. Your reasoning about it not being a security problem seems sound.
Fixed in git for 1.10.28, 1.12.10 and 1.13.6
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.