[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

X Strike Force SVN commit: rev 471 - in branches/4.1.0/woody/debian: . patches



Author: branden
Date: 2003-09-02 18:49:27 -0500 (Tue, 02 Sep 2003)
New Revision: 471

Added:
   branches/4.1.0/woody/debian/patches/071_SECURITY_improved_MIT-SHM_fix.diff
Modified:
   branches/4.1.0/woody/debian/changelog
Log:
debian/patches/071_SECURITY_improved_MIT-SHM_fix.diff: new; updated fix to
  MIT-SHM vulnerability from upstream CVS:
  - Implement LocalClientCred() to return the credentials of local clients
    connected through Unix domain sockets on systems that have the required
    support (for now recent Linux, FreeBSD >= 4.6, OpenBSD >= 3.0 are
    implemented), and use that in ShmAttach() to grant access to the client.
    When client credentials are not available, require world accessibility.
  The original fix did not cover the case where the X server is started
  from an X display manager such as xdm.


Modified: branches/4.1.0/woody/debian/changelog
===================================================================
--- branches/4.1.0/woody/debian/changelog	2003-09-02 22:51:58 UTC (rev 470)
+++ branches/4.1.0/woody/debian/changelog	2003-09-02 23:49:27 UTC (rev 471)
@@ -3,14 +3,26 @@
   * Security update release.  Resolves the following issues:
     + CAN-2003-0063 (xterm window title reporting can deceive user)
     + CAN-2003-0071 (xterm susceptible to DEC UDK sequence DoS attack)
+    + CAN-2002-0164 (flaw in X server's MIT-SHM extension can user owning X
+                    session to read and write arbitrary shared memory
+                    segments)
 
   * patch #069: new; disable xterm's window title reporting escape sequence
 
   * patch #070: new; fix term to ignore malformed DEC UDK escape sequences
     instead of locking up
 
- -- Branden Robinson <branden@debian.org>  Tue,  2 Sep 2003 14:34:48 -0500
+  * patch #071: new; updated fix to MIT-SHM vulnerability from upstream CVS:
+    - Implement LocalClientCred() to return the credentials of local clients
+      connected through Unix domain sockets on systems that have the required
+      support (for now recent Linux, FreeBSD >= 4.6, OpenBSD >= 3.0 are
+      implemented), and use that in ShmAttach() to grant access to the client.
+      When client credentials are not available, require world accessibility.
+    The original fix did not cover the case where the X server is started
+    from an X display manager such as xdm.
 
+ -- Branden Robinson <branden@debian.org>  Tue,  2 Sep 2003 18:43:32 -0500
+
 xfree86 (4.1.0-16) unstable; urgency=high
 
   * patch #000_stolen_from_HEAD:

Added: branches/4.1.0/woody/debian/patches/071_SECURITY_improved_MIT-SHM_fix.diff
===================================================================
--- branches/4.1.0/woody/debian/patches/071_SECURITY_improved_MIT-SHM_fix.diff	2003-09-02 22:51:58 UTC (rev 470)
+++ branches/4.1.0/woody/debian/patches/071_SECURITY_improved_MIT-SHM_fix.diff	2003-09-02 23:49:27 UTC (rev 471)
@@ -0,0 +1,277 @@
+Implement LocalClientCred() to return the credentials of local clients
+connected through Unix domain sockets on systems that have the required
+support (for now recent Linux, FreeBSD >= 4.6, OpenBSD >= 3.0 are
+implemented), and use that in ShmAttach() to grant access to the client.
+When client credentials are not available, require world accessibility.
+
+This patch from XFree86 CVS.
+
+--- xc/config/cf/Imake.tmpl~	2003-09-02 15:53:44.000000000 -0500
++++ xc/config/cf/Imake.tmpl	2003-09-02 15:58:55.000000000 -0500
+@@ -378,6 +378,9 @@
+ #ifndef HasPamMisc
+ #define HasPamMisc		NO
+ #endif
++#ifndef HasGetpeereid
++#define HasGetpeereid		NO
++#endif
+ /* byte-order defaults */
+ #ifndef ByteOrder
+ #if defined(VaxArchitecture)
+--- xc/doc/specs/Xext/mit-shm.ms	1994-04-27 02:25:28.000000000 -0500
++++ xc/doc/specs/Xext/mit-shm.ms	2002-09-13 01:40:56.000000000 -0500
+@@ -213,6 +213,13 @@
+ shminfo structure.  The server will need that ID to attach itself to the
+ segment.
+ .LP
++Also note that, on many systems for security reasons, the X server
++will only accept to attach to the shared memory segment if it's
++readable and writeable by ``other''. On systems where the X server is
++able to determine the uid of the X client over a local transport, the
++shared memory segment can be readable and writeable only by the uid of
++the client.
++.LP
+ Next, attach this shared memory segment to your process:
+ .Cs
+ shminfo.shmaddr = image->data = shmat (shminfo.shmid, 0, 0);
+--- xc/programs/Xserver/Xext/shm.c~	2003-09-02 18:37:00.000000000 -0500
++++ xc/programs/Xserver/Xext/shm.c	2003-09-02 18:39:07.000000000 -0500
+@@ -33,6 +33,7 @@
+ #include <ipc.h>
+ #include <shm.h>
+ #endif
++#include <sys/stat.h>
+ #define NEED_REPLIES
+ #define NEED_EVENTS
+ #include "X.h"
+@@ -62,12 +63,6 @@
+ extern PanoramiXData   *panoramiXdataPtr;
+ #endif
+ 
+-#if defined(SVR4) || defined(__linux__) || defined(CSRG_BASED)
+-#define HAS_SAVED_IDS_AND_SETEUID
+-#else
+-#include <sys/stat.h>
+-#endif
+-
+ typedef struct _ShmDesc {
+     struct _ShmDesc *next;
+     int shmid;
+@@ -361,35 +356,38 @@
+     return (client->noClientException);
+ }
+ 
+-#ifndef HAS_SAVED_IDS_AND_SETEUID
+ /*
+  * Simulate the access() system call for a shared memory segement,
+- * using the real user and group id of the process
+- * /
++ * using the credentials from the client if available
++ */
+ static int
+-shm_access(uid_t uid, gid_t gid, struct ipc_perm *perm, int readonly)
++shm_access(ClientPtr client, struct ipc_perm *perm, int readonly)
+ {
++    int uid, gid;
+     mode_t mask;
+ 
+-    /* User id 0 always gets access */
+-    if (uid == 0) {
+-	return 0;
+-    }
+-    /* Check the owner */
+-    if (perm->uid == uid || perm->cuid == uid) {
+-	mask = S_IRUSR;
+-	if (!readonly) {
+-	    mask |= S_IWUSR;
++    if (LocalClientCred(client, &uid, &gid) != -1) {
++
++	/* User id 0 always gets access */
++	if (uid == 0) {
++	    return 0;
+ 	}
+-	return (perm->mode & mask) == mask ? 0 : -1;
+-    }
+-    /* Check the group */
+-    if (perm->gid == gid || perm->cgid == gid) {
+-	mask = S_IRGRP;
+-	if (!readonly) {
+-	    mask |= S_IWGRP;
++	/* Check the owner */
++	if (perm->uid == uid || perm->cuid == uid) {
++	    mask = S_IRUSR;
++	    if (!readonly) {
++		mask |= S_IWUSR;
++	    }
++	    return (perm->mode & mask) == mask ? 0 : -1;
++	}
++	/* Check the group */
++	if (perm->gid == gid || perm->cgid == gid) {
++	    mask = S_IRGRP;
++	    if (!readonly) {
++		mask |= S_IWGRP;
++	    }
++	    return (perm->mode & mask) == mask ? 0 : -1;
+ 	}
+-	return (perm->mode & mask) == mask ? 0 : -1;
+     }
+     /* Otherwise, check everyone else */
+     mask = S_IROTH;
+@@ -398,7 +396,6 @@
+     }
+     return (perm->mode & mask) == mask ? 0 : -1;
+ }
+-#endif
+ 
+ static int
+ ProcShmAttach(client)
+@@ -409,10 +406,8 @@
+     REQUEST(xShmAttachReq);
+     uid_t ruid;
+     gid_t rgid;
+-#ifdef HAS_SAVED_IDS_AND_SETEUID
+     uid_t euid;
+     gid_t egid;
+-#endif
+ 
+     REQUEST_SIZE_MATCH(xShmAttachReq);
+     LEGAL_NEW_RESOURCE(stuff->shmseg, client);
+@@ -438,7 +433,6 @@
+ 	    return BadAlloc;
+ 	ruid = getuid();
+ 	rgid = getgid();
+-#ifdef HAS_SAVED_IDS_AND_SETEUID
+ 	euid = geteuid();
+ 	egid = getegid();
+ 
+@@ -448,32 +442,31 @@
+ 		return BadAccess;
+ 	    }
+ 	}
+-#endif
+ 	shmdesc->addr = shmat(stuff->shmid, 0,
+ 			      stuff->readOnly ? SHM_RDONLY : 0);
+-#ifdef HAS_SAVED_IDS_AND_SETEUID
+ 	if (euid != ruid || egid != rgid) {
+ 	    /* Switch back to root privs */
+ 	    if (seteuid(euid) == -1 || setegid(egid) == -1) {
+ 		return BadAccess;
+ 	    }
+-	} 
+-#endif
++	}
+ 	if ((shmdesc->addr == ((char *)-1)) ||
+ 	    shmctl(stuff->shmid, IPC_STAT, &buf))
+ 	{
+ 	    xfree(shmdesc);
+ 	    return BadAccess;
+ 	}
+-#ifndef HAS_SAVED_IDS_AND_SETEUID
++
+ 	/* The attach was performed with root privs. We must
+-	 * do manual checking of access rights for the real uid/gid */
+-	if (shm_access(ruid, rgid, &(buf.shm_perm), stuff->readOnly) == -1) {
++	 * do manual checking of access rights for the credentials
++	 * of the client */
++
++	if (shm_access(client, &(buf.shm_perm), stuff->readOnly) == -1) {
+ 	    shmdt(shmdesc->addr);
+ 	    xfree(shmdesc);
+ 	    return BadAccess;
+ 	}
+-#endif	
++
+ 	shmdesc->shmid = stuff->shmid;
+ 	shmdesc->refcnt = 1;
+ 	shmdesc->writable = !stuff->readOnly;
+--- xc/programs/Xserver/include/os.h~	2003-09-02 16:10:55.000000000 -0500
++++ xc/programs/Xserver/include/os.h	2003-09-02 18:09:05.000000000 -0500
+@@ -625,6 +625,8 @@
+ #endif
+ );
+ 
++extern int LocalClientCred(ClientPtr, int *, int *);
++
+ extern int ChangeAccessControl(
+ #if NeedFunctionPrototypes
+     ClientPtr /*client*/,
+--- xc/programs/Xserver/os/Imakefile~	2003-09-02 18:20:29.000000000 -0500
++++ xc/programs/Xserver/os/Imakefile	2003-09-02 18:21:44.000000000 -0500
+@@ -83,6 +83,10 @@
+ MALLOC_OBJS=xalloc.o
+ #endif
+ 
++#if HasGetpeereid
++GETPEEREID_DEFINES = -DHAS_GETPEEREID
++#endif
++
+ BOOTSTRAPCFLAGS = 
+            SRCS = WaitFor.c access.c connection.c io.c $(COLOR_SRCS) \
+                   osinit.c utils.c auth.c mitauth.c secauth.c $(XDMAUTHSRCS) \
+@@ -114,7 +118,7 @@
+ #if HasPam && HasPamMisc
+     PAM_DEFINES = -DUSE_PAM
+ #endif
+-        DEFINES = -DXSERV_t -DTRANS_SERVER $(CONNECTION_FLAGS) $(MEM_DEFINES) $(XDMAUTHDEFS) $(RPCDEFS) $(SIGNAL_DEFINES) $(OS_DEFINES) $(KRB5_DEFINES) $(RGB_DEFINES)
++        DEFINES = -DXSERV_t -DTRANS_SERVER $(CONNECTION_FLAGS) $(MEM_DEFINES) $(XDMAUTHDEFS) $(RPCDEFS) $(SIGNAL_DEFINES) $(OS_DEFINES) $(KRB5_DEFINES) $(RGB_DEFINES) $(GETPEEREID_DEFINES)
+        INCLUDES = -I.  -I../include -I$(XINCLUDESRC) -I$(EXTINCSRC) -I$(TOP)/lib/Xau -I../lbx Krb5Includes
+  DEPEND_DEFINES = $(DBM_DEFINES) $(XDMCP_DEFINES) $(EXT_DEFINES) $(TRANS_INCLUDES) $(CONNECTION_FLAGS) DependDefines
+        LINTLIBS = ../dix/llib-ldix.ln
+--- xc/programs/Xserver/os/access.c~	2003-09-02 18:26:45.000000000 -0500
++++ xc/programs/Xserver/os/access.c	2003-09-02 18:27:53.000000000 -0500
+@@ -1033,6 +1033,55 @@
+     return FALSE;
+ }
+ 
++/*
++ * Return the uid and gid of a connected local client
++ * or the uid/gid for nobody those ids cannot be determinded
++ *
++ * Used by XShm to test access rights to shared memory segments
++ */
++int
++LocalClientCred(ClientPtr client, int *pUid, int *pGid)
++{
++    int fd;
++    XtransConnInfo ci;
++#ifdef HAS_GETPEEREID
++    uid_t uid;
++    gid_t gid;
++#elif defined(SO_PEERCRED)
++    struct ucred peercred;
++    socklen_t so_len = sizeof(peercred);
++#endif
++
++    if (client == NULL)
++	return -1;
++    ci = ((OsCommPtr)client->osPrivate)->trans_conn;
++    /* We can only determine peer credentials for Unix domain sockets */
++    if (!_XSERVTransIsLocal(ci)) {
++	return -1;
++    }
++    fd = _XSERVTransGetConnectionNumber(ci);
++#ifdef HAS_GETPEEREID
++    if (getpeereid(fd, &uid, &gid) == -1)
++	    return -1;
++    if (pUid != NULL)
++	    *pUid = uid;
++    if (pGid != NULL)
++	    *pGid = gid;
++    return 0;
++#elif defined(SO_PEERCRED)
++    if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &so_len) == -1)
++	    return -1;
++    if (pUid != NULL)
++	    *pUid = peercred.uid;
++    if (pGid != NULL)
++	    *pGid = peercred.gid;
++    return 0;
++#else
++    /* No system call available to get the credentials of the peer */
++    return -1;
++#endif
++}
++
+ static Bool
+ AuthorizedClient(ClientPtr client)
+ {



Reply to: