[PATCH 1/8] vchan-socket-proxy: Ensure UNIX path NUL terminated

Jason Andryuk posted 8 patches 5 years, 8 months ago
Maintainers: Wei Liu <wl@xen.org>, Ian Jackson <ian.jackson@eu.citrix.com>
There is a newer version of this series
[PATCH 1/8] vchan-socket-proxy: Ensure UNIX path NUL terminated
Posted by Jason Andryuk 5 years, 8 months ago
Check the socket path length to ensure sun_path is NUL terminated.

This was spotted by Citrix's Coverity.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 tools/libvchan/vchan-socket-proxy.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
index 13700c5d67..6d860af340 100644
--- a/tools/libvchan/vchan-socket-proxy.c
+++ b/tools/libvchan/vchan-socket-proxy.c
@@ -148,6 +148,12 @@ static int connect_socket(const char *path_or_fd) {
         return fd;
     }
 
+    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
+        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
+                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
+        return -1;
+    }
+
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd == -1)
         return -1;
@@ -174,6 +180,12 @@ static int listen_socket(const char *path_or_fd) {
         return fd;
     }
 
+    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
+        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
+                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
+        return -1;
+    }
+
     /* if not a number, assume a socket path */
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd == -1)
-- 
2.25.1


Re: [PATCH 1/8] vchan-socket-proxy: Ensure UNIX path NUL terminated
Posted by Marek Marczykowski-Górecki 5 years, 8 months ago
On Sun, May 24, 2020 at 10:49:48PM -0400, Jason Andryuk wrote:
> Check the socket path length to ensure sun_path is NUL terminated.
> 
> This was spotted by Citrix's Coverity.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
>  tools/libvchan/vchan-socket-proxy.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/libvchan/vchan-socket-proxy.c b/tools/libvchan/vchan-socket-proxy.c
> index 13700c5d67..6d860af340 100644
> --- a/tools/libvchan/vchan-socket-proxy.c
> +++ b/tools/libvchan/vchan-socket-proxy.c
> @@ -148,6 +148,12 @@ static int connect_socket(const char *path_or_fd) {
>          return fd;
>      }
>  
> +    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
> +        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
> +                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
> +        return -1;
> +    }
> +
>      fd = socket(AF_UNIX, SOCK_STREAM, 0);
>      if (fd == -1)
>          return -1;
> @@ -174,6 +180,12 @@ static int listen_socket(const char *path_or_fd) {
>          return fd;
>      }
>  
> +    if (strlen(path_or_fd) >= sizeof(addr.sun_path)) {
> +        fprintf(stderr, "UNIX socket path \"%s\" too long (%zd >= %zd)\n",
> +                path_or_fd, strlen(path_or_fd), sizeof(addr.sun_path));
> +        return -1;
> +    }
> +
>      /* if not a number, assume a socket path */
>      fd = socket(AF_UNIX, SOCK_STREAM, 0);
>      if (fd == -1)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?