Attachment 140793 Details for Bug 107350 – sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un

[patch] sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un

0001-sysdeps-Reassure-gcc-8-that-we-are-not-overflowing-s.patch (text/plain), 3.41 KB, created by Simon McVittie on 2018-07-23 17:58:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2018-07-23 17:58:34 UTC

Size: 3.41 KB

patch

obsolete

>From 218f7aee108381cd076062b161eea02af7ab8903 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>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 <smcv@collabora.com>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 107350: 140793

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.