diff options
| -rw-r--r-- | ChangeLog | 30 | ||||
| -rw-r--r-- | unix/tkUnixSend.c | 84 |
2 files changed, 70 insertions, 44 deletions
@@ -1,17 +1,23 @@ +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 Jan Nijtmans <nijtmans@users.sf.net> - * generic/tkListbox.c: Fix gcc warning: ignoring return value of ‘strtol’, - declared with attribute warn_unused_result. - * unix/tkUnixEvent.c: Fix gcc warning: dereferencing pointer ‘xgePtr’ does - break strict-aliasing rules. - * generic/tkInt.decls: CONSTify return values of TkKeysymToString, - * generic/tkBind.c TkFindStateString, TkpGetString, TkpGetChar, - * generic/tkIntDecls.h which are all not supposed to be modified by - * generic/tkUtil.c the caller. In tkUtil.c this gets rid of a - * carbon/tkMacOSXKeyboard.c dangerous type cast. - * macosx/tkMacOSXKeyboard.c - * unix/tkUnixKey.c - * win/tkWinKey.c + * generic/tkListbox.c: Fix gcc warning: ignoring return value of + strtol, declared with attribute + warn_unused_result. + * unix/tkUnixEvent.c: Fix gcc warning: dereferencing pointer xgePtr + does break strict-aliasing rules. + * generic/tkInt.decls: CONSTify return values of TkKeysymToString, + * generic/tkBind.c: TkFindStateString, TkpGetString, TkpGetChar, + * generic/tkIntDecls.h: which are all not supposed to be modified by + * generic/tkUtil.c: the caller. In tkUtil.c this gets rid of a + * carbon/tkMacOSXKeyboard.c: dangerous type cast. + * macosx/tkMacOSXKeyboard.c: + * unix/tkUnixKey.c: + * win/tkWinKey.c: 2009-12-15 Don Porter <dgp@users.sourceforge.net> diff --git a/unix/tkUnixSend.c b/unix/tkUnixSend.c index b6d4dce..108d109 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.25 2009/08/25 08:46:06 dkf Exp $ + * RCS: @(#) $Id: tkUnixSend.c,v 1.26 2009/12/20 23:26:53 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. + */ + + secure = 0; +#endif /* FamilyServerInterpreted */ } if (addrPtr != NULL) { XFree((char *) addrPtr); @@ -736,7 +756,7 @@ ServerSecure( } /* - *-------------------------------------------------------------- + *---------------------------------------------------------------------- * * Tk_SetAppName -- * @@ -757,7 +777,7 @@ ServerSecure( * registration will be removed automatically if the interpreter is * deleted or the "send" command is removed. * - *-------------------------------------------------------------- + *---------------------------------------------------------------------- */ const char * |