Apply (a version of) [Patch 2917663].

2 files changed, 62 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index 9cea675..34fbed8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2009-12-20  Donal K. Fellows  <dkf@users.sf.net>
+
+	* unix/tkUnixSend.c (ServerSecure): [Patch 2917663]: Better support
+	for server-interpreted access control addreses.
+
 2009-12-16  Joe English  <jenglish@users.sourceforge.net>
 
 	* generic/ttk/ttkNotebook.c: Don't call Tk_DeleteOptionTable()
@@ -5,18 +10,17 @@
 
 2009-12-14  Kevin B. Kenny  <kennykb@acm.org>
 
-	* library/demos/unicodeout.tcl: Added code to check for
-	right-to-left support on Windows and adjust Hebrew and Arabic
-	character strings accordingly. Changed the Hebrew string to
-	'ktb ebryt' (ktav Ivrit, "Hebrew writing") to be consistent
-	with at least the Greek and Russian strings. Thanks to
-	Rodrigo Readi for calling the inconsistency to our attention.
+	* library/demos/unicodeout.tcl: Added code to check for right-to-left
+	support on Windows and adjust Hebrew and Arabic character strings
+	accordingly. Changed the Hebrew string to 'ktb ebryt' (ktav Ivrit,
+	"Hebrew writing") to be consistent with at least the Greek and Russian
+	strings. Thanks to Rodrigo Readi for calling the inconsistency to our
+	attention.
 	
 2009-12-02  Jan Nijtmans  <nijtmans@users.sf.net>
 
 	* win/tkInt.decls:	[Bugs 220600, 220690]: Comment that
-	  TkWinChildProc is exported through the stubs table
-	  since 8.5.9
+	TkWinChildProc is exported through the stubs table since 8.5.9
 
 2009-12-11  Donal K. Fellows  <dkf@users.sf.net>
 
diff --git a/unix/tkUnixSend.c b/unix/tkUnixSend.c
index 1c8c67c..711315a 100644
--- a/unix/tkUnixSend.c
+++ b/unix/tkUnixSend.c
@@ -11,7 +11,7 @@
  * See the file "license.terms" for information on usage and redistribution of
  * this file, and for a DISCLAIMER OF ALL WARRANTIES.
  *
- * RCS: @(#) $Id: tkUnixSend.c,v 1.20.2.1 2009/08/25 08:48:16 dkf Exp $
+ * RCS: @(#) $Id: tkUnixSend.c,v 1.20.2.2 2009/12/20 23:16:30 dkf Exp $
  */
 
 #include "tkUnixInt.h"
@@ -679,54 +679,74 @@ ServerSecure(
     int numHosts, secure;
     Bool enabled;
 
-    secure = 0;
     addrPtr = XListHosts(dispPtr->display, &numHosts, &enabled);
-    if (enabled) {
-	if (numHosts == 0) {
-	    secure = 1;
-	}
-
+    if (!enabled) {
+    insecure:
+	secure = 0;
+    } else if (numHosts == 0) {
+	secure = 1;
+    } else {
 	/*
 	 * Recent versions of X11 have the extra feature of allowing more
 	 * sophisticated authorization checks to be performed than the dozy
 	 * old ones that used to plague xhost usage. However, not all deployed
 	 * versions of Xlib know how to deal with this feature, so this code
 	 * is conditional on having the right #def in place. [Bug 1909931]
+	 *
+	 * Note that at this point we know that there's at least one entry in
+	 * the list returned by XListHosts. However there may be multiple
+	 * entries; as long as each is one of either 'SI:localhost:*' or
+	 * 'SI:localgroup:*' then we will claim to be secure enough.
 	 */
 
 #ifdef FamilyServerInterpreted
-	if (numHosts == 1 && addrPtr[0].family == FamilyServerInterpreted) {
-	    XServerInterpretedAddress *siPtr =
-		    (XServerInterpretedAddress *) addrPtr[0].address;
-
-	    if (siPtr->typelength==9 && !memcmp(siPtr->type,"localuser",9)) {
-		/*
-		 * We don't check the username here. This is because it's
-		 * officially non-portable and we are just making sure there
-		 * aren't silly misconfigurations. (Apparently 'root' is not a
-		 * very good choice, but we still don't put any effort in to
-		 * spot that.)
-		 */
+	XServerInterpretedAddress *siPtr;
+	int i;
 
-		secure = 1;
-	    } else if (siPtr->typelength == 10
-		    && !memcmp(siPtr->type, "localgroup", 10)) {
+	for (i=0 ; i<numHosts ; i++) {
+	    if (addrPtr[i].family != FamilyServerInterpreted) {
 		/*
-		 * Similarly to above, we don't attempt to peek inside server
-		 * interpreted group names. If someone set it, it's what they
-		 * want and we assume it's OK.
+		 * We don't understand what the X server is letting in, so we
+		 * err on the side of safety.
 		 */
 
-		secure = 1;
+		goto insecure;
 	    }
+	    siPtr = (XServerInterpretedAddress *) addrPtr[0].address;
 
 	    /*
-	     * The other defined types of server-interpreted controls involve
-	     * particular hosts; these are still insecure for the same reasons
-	     * that classic xhost access is insecure.
+	     * We don't check the username or group here. This is because it's
+	     * officially non-portable and we are just making sure there
+	     * aren't silly misconfigurations. (Apparently 'root' is not a
+	     * very good choice, but we still don't put any effort in to spot
+	     * that.) However we do check to see that the constraints are
+	     * imposed against the connecting user and/or group.
 	     */
+
+	    if (       !(siPtr->typelength == 9 /* ==strlen("localuser") */
+			&& !memcmp(siPtr->type, "localuser", 9))
+		    && !(siPtr->typelength == 10 /* ==strlen("localgroup") */
+			&& !memcmp(siPtr->type, "localgroup", 10))) {
+		/*
+		 * The other defined types of server-interpreted controls
+		 * involve particular hosts. These are still insecure for the
+		 * same reasons that classic xhost access is insecure; there's
+		 * just no way to be sure that the users on those systems are
+		 * the ones who should be allowed to connect to this display.
+		 */
+
+		goto insecure;
+	    }
 	}
-#endif
+	secure = 1;
+#else
+	/*
+	 * We don't understand what the X server is letting in, so we err on
+	 * the side of safety.
+	 */
+
+	goto insecure;
+#endif /* FamilyServerInterpreted */
     }
     if (addrPtr != NULL) {
 	XFree((char *) addrPtr);