From b5545a7b7e98f79c4c3b4b6e735c77ed2b78b9d2 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <corinna@vinschen.de>
Date: Thu, 7 Mar 2013 11:04:28 +0000
Subject: [PATCH] 	* fhandler_socket.cc (fhandler_socket::bind): Fix
 check for AF_LOCAL 	filename length to allow non-NUL terminated strings
 within namelen 	bytes.  Copy over sun_path to local array sun_path to
 have a 	NUL-terminated string for subsequent function calls.  Move
 path_conv 	check before OS bind call to not bind the socket before being
 sure 	the file doesn't exist.  Add and fix comments.

---
 winsup/cygwin/ChangeLog          |  9 +++++++
 winsup/cygwin/fhandler_socket.cc | 43 +++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index b9c5a62a7..11299dd2b 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,12 @@
+2013-03-07  Corinna Vinschen  <corinna@vinschen.de>
+
+	* fhandler_socket.cc (fhandler_socket::bind): Fix check for AF_LOCAL
+	filename length to allow non-NUL terminated strings within namelen
+	bytes.  Copy over sun_path to local array sun_path to have a
+	NUL-terminated string for subsequent function calls.  Move path_conv
+	check before OS bind call to not bind the socket before being sure
+	the file doesn't exist.  Add and fix comments.
+
 2013-03-06  Corinna Vinschen  <corinna@vinschen.de>
 
 	* mount.cc (fs_names): Add trailing NULL element to avoid potential
diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc
index 8b05f75da..d52f75901 100644
--- a/winsup/cygwin/fhandler_socket.cc
+++ b/winsup/cygwin/fhandler_socket.cc
@@ -902,13 +902,37 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
       struct sockaddr_in sin;
       int len = namelen - offsetof (struct sockaddr_un, sun_path);
 
-      /* Check that name is within bounds and NUL-terminated. */
-      if (len <= 1 || len > UNIX_PATH_LEN
-	  || !memchr (un_addr->sun_path, '\0', len))
+      /* Check that name is within bounds.  Don't check if the string is
+         NUL-terminated, because there are projects out there which set
+	 namelen to a value which doesn't cover the trailing NUL. */
+      if (len <= 1 || (len = strnlen (un_addr->sun_path, len)) > UNIX_PATH_LEN)
 	{
 	  set_errno (len <= 1 ? (len == 1 ? ENOENT : EINVAL) : ENAMETOOLONG);
 	  goto out;
 	}
+      /* Copy over the sun_path string into a buffer big enough to add a
+	 trailing NUL. */
+      char sun_path[len + 1];
+      strncpy (sun_path, un_addr->sun_path, len);
+      sun_path[len] = '\0';
+
+      /* This isn't entirely foolproof, but we check first if the file exists
+	 so we can return with EADDRINUSE before having bound the socket.
+	 This allows an application to call bind again on the same socket using
+	 another filename.  If we bind first, the application will not be able
+	 to call bind successfully ever again. */
+      path_conv pc (sun_path, PC_SYM_FOLLOW);
+      if (pc.error)
+	{
+	  set_errno (pc.error);
+	  goto out;
+	}
+      if (pc.exists ())
+	{
+	  set_errno (EADDRINUSE);
+	  goto out;
+	}
+
       sin.sin_family = AF_INET;
       sin.sin_port = 0;
       sin.sin_addr.s_addr = htonl (INADDR_LOOPBACK);
@@ -928,17 +952,6 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
       sin.sin_port = ntohs (sin.sin_port);
       debug_printf ("AF_LOCAL: socket bound to port %u", sin.sin_port);
 
-      path_conv pc (un_addr->sun_path, PC_SYM_FOLLOW);
-      if (pc.error)
-	{
-	  set_errno (pc.error);
-	  goto out;
-	}
-      if (pc.exists ())
-	{
-	  set_errno (EADDRINUSE);
-	  goto out;
-	}
       mode_t mode = adjust_socket_file_mode ((S_IRWXU | S_IRWXG | S_IRWXO)
 					     & ~cygheap->umask);
       DWORD fattr = FILE_ATTRIBUTE_SYSTEM;
@@ -999,7 +1012,7 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen)
 	    }
 	  else
 	    {
-	      set_sun_path (un_addr->sun_path);
+	      set_sun_path (sun_path);
 	      res = 0;
 	    }
 	  NtClose (fh);