[<prev] [next>] [<thread-prev] [month] [year] [list]
Date: Thu, 3 Oct 2002 16:36:38 -0300
From: "Ademar de Souza Reis Jr." <ademar@...ectiva.com.br>
To: xvendor@...ts.openwall.com
Subject: Re: XFree86 MIT-SHM vulnerability
On Wed, Oct 02, 2002 at 06:27:12PM -0300, Ademar de Souza Reis Jr. wrote:
> Anyway, the new patch mentioned definetely fails to apply in 4.0.3.
> I didn't investigate it and if someone is working on it, please tell me
> (otherwise I'll do it by myself :-).
Actually the backport was very easy...
Patches for 4.0.3 and 4.0.1 attached FYI.
(I'm compiling XFree86 right now, they're not tested).
--
Ademar de Souza Reis Jr. <ademar@...ectiva.com.br>
^[:wq!
diff -ur XFree86-4.0.3-orig/xc/config/cf/FreeBSD.cf XFree86-4.0.3/xc/config/cf/FreeBSD.cf
--- XFree86-4.0.3-orig/xc/config/cf/FreeBSD.cf Fri Mar 2 19:55:19 2001
+++ XFree86-4.0.3/xc/config/cf/FreeBSD.cf Thu Oct 3 14:32:49 2002
@@ -131,6 +131,10 @@
#define HasSetUserContext YES
#endif
+#if OSMajorVersion >= 5 || (OSMajorVersion == 4 && OSMinorVersion >= 6)
+#define HasGetpeereid YES
+#endif
+
/* 3.3(?) and later has support for setting MTRRs */
#ifndef HasMTRRSupport
#if OSMajorVersion > 3 || (OSMajorVersion == 3 && OSMinorVersion >= 3)
diff -ur XFree86-4.0.3-orig/xc/config/cf/Imake.tmpl XFree86-4.0.3/xc/config/cf/Imake.tmpl
--- XFree86-4.0.3-orig/xc/config/cf/Imake.tmpl Mon Feb 26 15:21:36 2001
+++ XFree86-4.0.3/xc/config/cf/Imake.tmpl Thu Oct 3 14:32:49 2002
@@ -358,6 +358,9 @@
#ifndef HasPamMisc
#define HasPamMisc NO
#endif
+#ifndef HasGetpeereid
+#define HasGetpeereid NO
+#endif
/* byte-order defaults */
#ifndef ByteOrder
#if defined(VaxArchitecture)
diff -ur XFree86-4.0.3-orig/xc/config/cf/OpenBSD.cf XFree86-4.0.3/xc/config/cf/OpenBSD.cf
--- XFree86-4.0.3-orig/xc/config/cf/OpenBSD.cf Fri Mar 2 17:40:45 2001
+++ XFree86-4.0.3/xc/config/cf/OpenBSD.cf Thu Oct 3 14:33:35 2002
@@ -735,6 +735,11 @@
# endif
#endif
+/* OpenBSD 3.0 and later has getpeereid() */
+#if OSMajorVersion >= 3
+# define HasGetpeereid YES
+#endif
+
/* This must come last */
#if OSMajorVersion == 2 && OSMinorVersion >= 8 || OSMajorVersion > 2
# include <OpenBSDLib.rules>
diff -ur XFree86-4.0.3-orig/xc/doc/specs/Xext/mit-shm.ms XFree86-4.0.3/xc/doc/specs/Xext/mit-shm.ms
--- XFree86-4.0.3-orig/xc/doc/specs/Xext/mit-shm.ms Wed Apr 27 04:25:28 1994
+++ XFree86-4.0.3/xc/doc/specs/Xext/mit-shm.ms Thu Oct 3 14:32:49 2002
@@ -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);
diff -ur XFree86-4.0.3-orig/xc/programs/Xserver/Xext/shm.c XFree86-4.0.3/xc/programs/Xserver/Xext/shm.c
--- XFree86-4.0.3-orig/xc/programs/Xserver/Xext/shm.c Thu Aug 10 14:40:30 2000
+++ XFree86-4.0.3/xc/programs/Xserver/Xext/shm.c Thu Oct 3 14:46:34 2002
@@ -37,6 +37,7 @@
#include <ipc.h>
#include <shm.h>
#endif
+#include <sys/stat.h>
#define NEED_REPLIES
#define NEED_EVENTS
#include "X.h"
@@ -359,6 +360,46 @@
return (client->noClientException);
}
+ /*
+ * Simulate the access() system call for a shared memory segement,
+ * using the credentials from the client if available
+ */
+ static int
+ shm_access(ClientPtr client, struct ipc_perm *perm, int readonly)
+ {
+ int uid, gid;
+ mode_t mask;
+
+ if (LocalClientCred(client, &uid, &gid) != -1) {
+ /* 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;
+ }
+ 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;
+ }
+ }
+ /* Otherwise, check everyone else */
+ mask = S_IROTH;
+ if (!readonly) {
+ mask |= S_IWOTH;
+ }
+ return (perm->mode & mask) == mask ? 0 : -1;
+ }
+
static int
ProcShmAttach(client)
register ClientPtr client;
@@ -389,14 +430,26 @@
shmdesc = (ShmDescPtr) xalloc(sizeof(ShmDescRec));
if (!shmdesc)
return BadAlloc;
+
shmdesc->addr = shmat(stuff->shmid, 0,
stuff->readOnly ? SHM_RDONLY : 0);
+
if ((shmdesc->addr == ((char *)-1)) ||
shmctl(stuff->shmid, IPC_STAT, &buf))
{
xfree(shmdesc);
return BadAccess;
}
+
+ /* The attach was performed with root privs. We must
+ * 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;
+ }
+
shmdesc->shmid = stuff->shmid;
shmdesc->refcnt = 1;
shmdesc->writable = !stuff->readOnly;
diff -ur XFree86-4.0.3-orig/xc/programs/Xserver/include/os.h XFree86-4.0.3/xc/programs/Xserver/include/os.h
--- XFree86-4.0.3-orig/xc/programs/Xserver/include/os.h Thu Nov 30 21:30:06 2000
+++ XFree86-4.0.3/xc/programs/Xserver/include/os.h Thu Oct 3 14:32:49 2002
@@ -625,6 +625,8 @@
#endif
);
+extern int LocalClientCred(ClientPtr, int *, int *);
+
extern int ChangeAccessControl(
#if NeedFunctionPrototypes
ClientPtr /*client*/,
diff -ur XFree86-4.0.3-orig/xc/programs/Xserver/os/Imakefile XFree86-4.0.3/xc/programs/Xserver/os/Imakefile
--- XFree86-4.0.3-orig/xc/programs/Xserver/os/Imakefile Thu Nov 30 21:30:06 2000
+++ XFree86-4.0.3/xc/programs/Xserver/os/Imakefile Thu Oct 3 14:48:31 2002
@@ -78,6 +78,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) \
@@ -111,7 +115,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)
LINTLIBS = ../dix/llib-ldix.ln
diff -ur XFree86-4.0.3-orig/xc/programs/Xserver/os/access.c XFree86-4.0.3/xc/programs/Xserver/os/access.c
--- XFree86-4.0.3-orig/xc/programs/Xserver/os/access.c Sun Dec 26 22:39:57 1999
+++ XFree86-4.0.3/xc/programs/Xserver/os/access.c Thu Oct 3 14:32:49 2002
@@ -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)
{
diff -ur XFree86-4.0.1-orig/xc/config/cf/FreeBSD.cf XFree86-4.0.1/xc/config/cf/FreeBSD.cf
--- XFree86-4.0.1-orig/xc/config/cf/FreeBSD.cf Tue Aug 15 22:45:27 2000
+++ XFree86-4.0.1/xc/config/cf/FreeBSD.cf Thu Oct 3 15:47:10 2002
@@ -113,6 +113,10 @@
#define HasSetUserContext YES
#endif
+#if OSMajorVersion >= 5 || (OSMajorVersion == 4 && OSMinorVersion >= 6)
+#define HasGetpeereid YES
+#endif
+
/* 3.3(?) and later has support for setting MTRRs */
#ifndef HasMTRRSupport
#if OSMajorVersion > 3 || (OSMajorVersion == 3 && OSMinorVersion >= 3)
diff -ur XFree86-4.0.1-orig/xc/config/cf/Imake.tmpl XFree86-4.0.1/xc/config/cf/Imake.tmpl
--- XFree86-4.0.1-orig/xc/config/cf/Imake.tmpl Fri Aug 25 10:42:15 2000
+++ XFree86-4.0.1/xc/config/cf/Imake.tmpl Thu Oct 3 15:47:10 2002
@@ -336,6 +336,9 @@
#ifndef HasFchown
#define HasFchown YES
#endif
+#ifndef HasGetpeereid
+#define HasGetpeereid NO
+#endif
/* byte-order defaults */
#ifndef ByteOrder
#if defined(VaxArchitecture)
diff -ur XFree86-4.0.1-orig/xc/config/cf/OpenBSD.cf XFree86-4.0.1/xc/config/cf/OpenBSD.cf
--- XFree86-4.0.1-orig/xc/config/cf/OpenBSD.cf Tue Aug 1 20:38:45 2000
+++ XFree86-4.0.1/xc/config/cf/OpenBSD.cf Thu Oct 3 15:47:10 2002
@@ -649,6 +649,11 @@
# endif
#endif
+/* OpenBSD 3.0 and later has getpeereid() */
+#if OSMajorVersion >= 3
+# define HasGetpeereid YES
+#endif
+
/* This must come last */
#include <bsdLib.rules>
diff -ur XFree86-4.0.1-orig/xc/doc/specs/Xext/mit-shm.ms XFree86-4.0.1/xc/doc/specs/Xext/mit-shm.ms
--- XFree86-4.0.1-orig/xc/doc/specs/Xext/mit-shm.ms Wed Apr 27 04:25:28 1994
+++ XFree86-4.0.1/xc/doc/specs/Xext/mit-shm.ms Thu Oct 3 15:47:11 2002
@@ -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);
diff -ur XFree86-4.0.1-orig/xc/programs/Xserver/Xext/shm.c XFree86-4.0.1/xc/programs/Xserver/Xext/shm.c
--- XFree86-4.0.1-orig/xc/programs/Xserver/Xext/shm.c Thu Aug 10 14:40:30 2000
+++ XFree86-4.0.1/xc/programs/Xserver/Xext/shm.c Thu Oct 3 15:47:11 2002
@@ -37,6 +37,7 @@
#include <ipc.h>
#include <shm.h>
#endif
+#include <sys/stat.h>
#define NEED_REPLIES
#define NEED_EVENTS
#include "X.h"
@@ -359,6 +360,46 @@
return (client->noClientException);
}
+ /*
+ * Simulate the access() system call for a shared memory segement,
+ * using the credentials from the client if available
+ */
+ static int
+ shm_access(ClientPtr client, struct ipc_perm *perm, int readonly)
+ {
+ int uid, gid;
+ mode_t mask;
+
+ if (LocalClientCred(client, &uid, &gid) != -1) {
+ /* 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;
+ }
+ 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;
+ }
+ }
+ /* Otherwise, check everyone else */
+ mask = S_IROTH;
+ if (!readonly) {
+ mask |= S_IWOTH;
+ }
+ return (perm->mode & mask) == mask ? 0 : -1;
+ }
+
static int
ProcShmAttach(client)
register ClientPtr client;
@@ -389,14 +430,26 @@
shmdesc = (ShmDescPtr) xalloc(sizeof(ShmDescRec));
if (!shmdesc)
return BadAlloc;
+
shmdesc->addr = shmat(stuff->shmid, 0,
stuff->readOnly ? SHM_RDONLY : 0);
+
if ((shmdesc->addr == ((char *)-1)) ||
shmctl(stuff->shmid, IPC_STAT, &buf))
{
xfree(shmdesc);
return BadAccess;
}
+
+ /* The attach was performed with root privs. We must
+ * 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;
+ }
+
shmdesc->shmid = stuff->shmid;
shmdesc->refcnt = 1;
shmdesc->writable = !stuff->readOnly;
diff -ur XFree86-4.0.1-orig/xc/programs/Xserver/include/os.h XFree86-4.0.1/xc/programs/Xserver/include/os.h
--- XFree86-4.0.1-orig/xc/programs/Xserver/include/os.h Wed Aug 2 12:30:36 2000
+++ XFree86-4.0.1/xc/programs/Xserver/include/os.h Thu Oct 3 15:47:11 2002
@@ -624,6 +624,8 @@
#endif
);
+extern int LocalClientCred(ClientPtr, int *, int *);
+
extern int ChangeAccessControl(
#if NeedFunctionPrototypes
ClientPtr /*client*/,
diff -ur XFree86-4.0.1-orig/xc/programs/Xserver/os/Imakefile XFree86-4.0.1/xc/programs/Xserver/os/Imakefile
--- XFree86-4.0.1-orig/xc/programs/Xserver/os/Imakefile Fri May 5 14:47:29 2000
+++ XFree86-4.0.1/xc/programs/Xserver/os/Imakefile Thu Oct 3 15:48:00 2002
@@ -78,6 +78,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) \
@@ -108,7 +112,7 @@
KRB5_DEFINES = Krb5Defines
XALLOC_DEFINES = XallocDefines
ERROR_DEFINES = ServerErrorDefines
- 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)
LINTLIBS = ../dix/llib-ldix.ln
diff -ur XFree86-4.0.1-orig/xc/programs/Xserver/os/access.c XFree86-4.0.1/xc/programs/Xserver/os/access.c
--- XFree86-4.0.1-orig/xc/programs/Xserver/os/access.c Sun Dec 26 22:39:57 1999
+++ XFree86-4.0.1/xc/programs/Xserver/os/access.c Thu Oct 3 15:47:11 2002
@@ -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)
{
Please check out the
xvendor mailing list charter.
Hosted by DataForce ISP -
Powered by Openwall GNU/*/Linux