From 93615472844ddbf5c530ad911332e5f316aa21b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erkki=20Sepp=C3=A4l=C3=A4?= Date: Mon, 31 Jan 2011 14:02:06 +0200 Subject: Xrm: Handle the extremely unlikely situation of fstat failing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tracked variable "size" was passed to a negative sink. Reviewed-by: Alan Coopersmith Reviewed-by: Ander Conselvan de Oliveira Signed-off-by: Erkki Seppälä Signed-off-by: Alan Coopersmith (cherry picked from commit be3e6c205d94dedc1cdebf5d17b987f0f828377a) Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/Xrm.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nx-X11/lib/X11/Xrm.c b/nx-X11/lib/X11/Xrm.c index d88442c6e..b3d92bb48 100644 --- a/nx-X11/lib/X11/Xrm.c +++ b/nx-X11/lib/X11/Xrm.c @@ -1594,6 +1594,12 @@ ReadInFile(_Xconst char *filename) */ GetSizeOfFile(fd, size); + /* There might have been a problem trying to stat a file */ + if (size == -1) { + close (fd); + return (char *)NULL; + } + if (!(filebuf = Xmalloc(size + 1))) { /* leave room for '\0' */ close(fd); return (char *)NULL; -- cgit v1.2.3 From 290f94aea2b6cf0b265bce33cadcf2f2cbcacd53 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 11 Feb 2011 14:20:24 -0800 Subject: ximcp: Prevent memory leak & double free if multiple %L in string In the highly unlikely event that TransFileName was passed a path containing multiple %L entries, for each entry it would call _XlcFileName, leaking the previous results, and then for each entry it would copy from that pointer and free it, resulting in invalid pointers & possible double frees for each use after the first one freed it. Error: Use after free (CWE 416) Use after free of pointer 'lcCompose' at line 358 of nx-X11/lib/X11/imLcPrs.c in function 'TransFileName'. Previously freed at line 360 with free. Error: Use after free (CWE 416) Use after free of pointer 'lcCompose' at line 359 of nx-X11/lib/X11/imLcPrs.c in function 'TransFileName'. Previously freed at line 360 with free. Error: Double free (CWE 415) Double free of pointer 'lcCompose' at line 360 of nx-X11/lib/X11/imLcPrs.c in function 'TransFileName'. Previously freed at line 360 with free. [ This bug was found by the Parfait 0.3.6 bug checking tool. For more information see http://labs.oracle.com/projects/parfait/ ] Signed-off-by: Alan Coopersmith (cherry picked from commit 6ac417cea1136a3617f5e40f4b106aaa3f48d6c2) Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/imLcPrs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/imLcPrs.c b/nx-X11/lib/X11/imLcPrs.c index 4dbcbbed4..549fe523a 100644 --- a/nx-X11/lib/X11/imLcPrs.c +++ b/nx-X11/lib/X11/imLcPrs.c @@ -321,7 +321,8 @@ TransFileName(Xim im, char *name) l += strlen(home); break; case 'L': - lcCompose = _XlcFileName(im->core.lcd, COMPOSE_FILE); + if (lcCompose == NULL) + lcCompose = _XlcFileName(im->core.lcd, COMPOSE_FILE); if (lcCompose) l += strlen(lcCompose); break; @@ -357,7 +358,6 @@ TransFileName(Xim im, char *name) if (lcCompose) { strcpy(j, lcCompose); j += strlen(lcCompose); - Xfree(lcCompose); } break; case 'S': @@ -371,6 +371,7 @@ TransFileName(Xim im, char *name) } } *j = '\0'; + Xfree(lcCompose); return ret; } -- cgit v1.2.3 From e8ada07fa7dbbc88298e88a07f4b8613ec055cd8 Mon Sep 17 00:00:00 2001 From: Nickolai Zeldovich Date: Tue, 22 Jan 2013 10:03:00 -0500 Subject: XListFontsWithInfo: avoid accessing realloc'ed memory If exactly one of the two reallocs in XListFontsWithInfo() fails, the subsequent code accesses memory freed by the other realloc. Signed-off-by: Nickolai Zeldovich Reviewed-by: Alan Coopersmith Signed-off-by: Alan Coopersmith (cherry picked from commit deedeada53676ee529d700bf96fde0b29a3a1def) Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/FontInfo.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/nx-X11/lib/X11/FontInfo.c b/nx-X11/lib/X11/FontInfo.c index 368beebe4..fb296b8a8 100644 --- a/nx-X11/lib/X11/FontInfo.c +++ b/nx-X11/lib/X11/FontInfo.c @@ -90,6 +90,11 @@ XFontStruct **info) /* RETURN */ Xrealloc ((char *) flist, (unsigned) (sizeof(char *) * (size+1))); + if (tmp_finfo) + finfo = tmp_finfo; + if (tmp_flist) + flist = tmp_flist; + if ((! tmp_finfo) || (! tmp_flist)) { /* free all the memory that we allocated */ for (j=(i-1); (j >= 0); j--) { @@ -97,14 +102,10 @@ XFontStruct **info) /* RETURN */ if (finfo[j].properties) Xfree((char *) finfo[j].properties); } - if (tmp_flist) Xfree((char *) tmp_flist); - else Xfree((char *) flist); - if (tmp_finfo) Xfree((char *) tmp_finfo); - else Xfree((char *) finfo); + Xfree((char *) flist); + Xfree((char *) finfo); goto clearwire; } - finfo = tmp_finfo; - flist = tmp_flist; } else { if (! (finfo = (XFontStruct *) -- cgit v1.2.3 From de2d3cb6b870f3d9b8e3e5779415aad079b20633 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 21:05:27 -0800 Subject: integer overflow in _XQueryFont() on 32-bit platforms [CVE-2013-1981 1/13] If the CARD32 reply.nCharInfos * sizeof(XCharStruct) overflows an unsigned long, then too small of a buffer will be allocated for the data copied in from the reply. v2: Fix reply_left calculations, check calculated sizes fit in reply_left Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/Font.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/nx-X11/lib/X11/Font.c b/nx-X11/lib/X11/Font.c index f60a8a874..820b38944 100644 --- a/nx-X11/lib/X11/Font.c +++ b/nx-X11/lib/X11/Font.c @@ -31,6 +31,7 @@ authorization from the X Consortium and the XFree86 Project. #include #endif #include "Xlibint.h" +#include #if defined(XF86BIGFONT) #define USE_XF86BIGFONT @@ -183,7 +184,8 @@ _XQueryFont ( unsigned long seq) { register XFontStruct *fs; - register long nbytes; + unsigned long nbytes; + unsigned long reply_left; /* unused data words left in reply buffer */ xQueryFontReply reply; register xResourceReq *req; register _XExtension *ext; @@ -211,9 +213,10 @@ _XQueryFont ( } if (seq) DeqAsyncHandler(dpy, &async); + reply_left = reply.length - + ((SIZEOF(xQueryFontReply) - SIZEOF(xReply)) >> 2); if (! (fs = (XFontStruct *) Xmalloc (sizeof (XFontStruct)))) { - _XEatData(dpy, (unsigned long)(reply.nFontProps * SIZEOF(xFontProp) + - reply.nCharInfos * SIZEOF(xCharInfo))); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } fs->ext_data = NULL; @@ -239,32 +242,42 @@ _XQueryFont ( */ fs->properties = NULL; if (fs->n_properties > 0) { - nbytes = reply.nFontProps * sizeof(XFontProp); - fs->properties = (XFontProp *) Xmalloc ((unsigned) nbytes); + /* nFontProps is a CARD16 */ nbytes = reply.nFontProps * SIZEOF(xFontProp); + if ((nbytes >> 2) <= reply_left) { + size_t pbytes = reply.nFontProps * sizeof(XFontProp); + fs->properties = Xmalloc (pbytes); + } if (! fs->properties) { Xfree((char *) fs); - _XEatData(dpy, (unsigned long) - (nbytes + reply.nCharInfos * SIZEOF(xCharInfo))); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } _XRead32 (dpy, (long *)fs->properties, nbytes); + reply_left -= (nbytes >> 2); } /* * If no characters in font, then it is a bad font, but * shouldn't try to read nothing. */ + /* have to unpack charinfos on some machines (CRAY) */ fs->per_char = NULL; if (reply.nCharInfos > 0){ - nbytes = reply.nCharInfos * sizeof(XCharStruct); - if (! (fs->per_char = (XCharStruct *) Xmalloc ((unsigned) nbytes))) { + /* nCharInfos is a CARD32 */ + if (reply.nCharInfos < (INT_MAX / sizeof(XCharStruct))) { + nbytes = reply.nCharInfos * SIZEOF(xCharInfo); + if ((nbytes >> 2) <= reply_left) { + size_t cibytes = reply.nCharInfos * sizeof(XCharStruct); + fs->per_char = Xmalloc (cibytes); + } + } + if (! fs->per_char) { if (fs->properties) Xfree((char *) fs->properties); Xfree((char *) fs); - _XEatData(dpy, (unsigned long) - (reply.nCharInfos * SIZEOF(xCharInfo))); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } - nbytes = reply.nCharInfos * SIZEOF(xCharInfo); + _XRead16 (dpy, (char *)fs->per_char, nbytes); } -- cgit v1.2.3 From dda0c652f1ec6d935040daaf2755bf9f2f6d3c4b Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 21:05:27 -0800 Subject: integer overflow in _XF86BigfontQueryFont() [CVE-2013-1981 2/13] Similar to _XQueryFont, but with more ways to go wrong and overflow. Only compiled if libX11 is built with XF86BigFont support. v2: Fix reply_left calculations, check calculated sizes fit in reply_left Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/Font.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/nx-X11/lib/X11/Font.c b/nx-X11/lib/X11/Font.c index 820b38944..0501ae279 100644 --- a/nx-X11/lib/X11/Font.c +++ b/nx-X11/lib/X11/Font.c @@ -404,7 +404,8 @@ _XF86BigfontQueryFont ( unsigned long seq) { register XFontStruct *fs; - register long nbytes; + unsigned long nbytes; + unsigned long reply_left; /* unused data left in reply buffer */ xXF86BigfontQueryFontReply reply; register xXF86BigfontQueryFontReq *req; register _XExtension *ext; @@ -457,13 +458,10 @@ _XF86BigfontQueryFont ( DeqAsyncHandler(dpy, &async2); if (seq) DeqAsyncHandler(dpy, &async1); + reply_left = reply.length - + ((SIZEOF(xXF86BigfontQueryFontReply) - SIZEOF(xReply)) >> 2); if (! (fs = (XFontStruct *) Xmalloc (sizeof (XFontStruct)))) { - _XEatData(dpy, - reply.nFontProps * SIZEOF(xFontProp) - + (reply.nCharInfos > 0 && reply.shmid == (CARD32)(-1) - ? reply.nUniqCharInfos * SIZEOF(xCharInfo) - + (reply.nCharInfos+1)/2 * 2 * sizeof(CARD16) - : 0)); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } fs->ext_data = NULL; @@ -489,23 +487,32 @@ _XF86BigfontQueryFont ( */ fs->properties = NULL; if (fs->n_properties > 0) { - nbytes = reply.nFontProps * sizeof(XFontProp); - fs->properties = (XFontProp *) Xmalloc ((unsigned) nbytes); + /* nFontProps is a CARD16 */ nbytes = reply.nFontProps * SIZEOF(xFontProp); + if ((nbytes >> 2) <= reply_left) { + size_t pbytes = reply.nFontProps * sizeof(XFontProp); + fs->properties = Xmalloc (pbytes); + } if (! fs->properties) { Xfree((char *) fs); - _XEatData(dpy, - nbytes - + (reply.nCharInfos > 0 && reply.shmid == (CARD32)(-1) - ? reply.nUniqCharInfos * SIZEOF(xCharInfo) - + (reply.nCharInfos+1)/2 * 2 * sizeof(CARD16) - : 0)); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } _XRead32 (dpy, (long *)fs->properties, nbytes); + reply_left -= (nbytes >> 2); } fs->per_char = NULL; +#ifndef LONG64 + /* compares each part to half the maximum, which should be far more than + any real font needs, so the combined total doesn't overflow either */ + if (reply.nUniqCharInfos > ((ULONG_MAX / 2) / SIZEOF(xCharInfo)) || + reply.nCharInfos > ((ULONG_MAX / 2) / sizeof(CARD16))) { + Xfree((char *) fs); + _XEatDataWords(dpy, reply_left); + return (XFontStruct *)NULL; + } +#endif if (reply.nCharInfos > 0) { /* fprintf(stderr, "received font metrics, nCharInfos = %d, nUniqCharInfos = %d, shmid = %d\n", reply.nCharInfos, reply.nUniqCharInfos, reply.shmid); */ if (reply.shmid == (CARD32)(-1)) { @@ -519,14 +526,14 @@ _XF86BigfontQueryFont ( if (!pUniqCI) { if (fs->properties) Xfree((char *) fs->properties); Xfree((char *) fs); - _XEatData(dpy, nbytes); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } if (! (fs->per_char = (XCharStruct *) Xmalloc (reply.nCharInfos * sizeof(XCharStruct)))) { Xfree((char *) pUniqCI); if (fs->properties) Xfree((char *) fs->properties); Xfree((char *) fs); - _XEatData(dpy, nbytes); + _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; } _XRead16 (dpy, (char *) pUniqCI, nbytes); @@ -581,6 +588,7 @@ _XF86BigfontQueryFont ( if (!(extcodes->serverCapabilities & CAP_VerifiedLocal)) { struct shmid_ds buf; if (!(shmctl(reply.shmid, IPC_STAT, &buf) >= 0 + && reply.nCharInfos < (LONG_MAX / sizeof(XCharStruct)) && buf.shm_segsz >= reply.shmsegoffset + reply.nCharInfos * sizeof(XCharStruct) + sizeof(CARD32) && *(CARD32 *)(addr + reply.shmsegoffset + reply.nCharInfos * sizeof(XCharStruct)) == extcodes->serverSignature)) { shmdt(addr); -- cgit v1.2.3 From 29779559c92c3058edc298ca0a6e59e1293262b6 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 22:49:01 -0800 Subject: integer overflow in XListFontsWithInfo() [CVE-2013-1981 3/13] If the reported number of remaining fonts is too large, the calculations to allocate memory for them may overflow, leaving us writing beyond the bounds of the allocation. v2: Fix reply_left calculations, check calculated sizes fit in reply_left v3: On error cases, also set values to be returned in pointer args to 0/NULL Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/FontInfo.c | 105 ++++++++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/nx-X11/lib/X11/FontInfo.c b/nx-X11/lib/X11/FontInfo.c index fb296b8a8..4b295a5eb 100644 --- a/nx-X11/lib/X11/FontInfo.c +++ b/nx-X11/lib/X11/FontInfo.c @@ -28,6 +28,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include #if defined(XF86BIGFONT) #define USE_XF86BIGFONT @@ -45,10 +46,11 @@ int maxNames, int *actualCount, /* RETURN */ XFontStruct **info) /* RETURN */ { - register long nbytes; + unsigned long nbytes; + unsigned long reply_left; /* unused data left in reply buffer */ register int i; register XFontStruct *fs; - register int size = 0; + unsigned int size = 0; XFontStruct *finfo = NULL; char **flist = NULL; xListFontsWithInfoReply reply; @@ -67,52 +69,44 @@ XFontStruct **info) /* RETURN */ if (!_XReply (dpy, (xReply *) &reply, ((SIZEOF(xListFontsWithInfoReply) - SIZEOF(xGenericReply)) >> 2), xFalse)) { - for (j=(i-1); (j >= 0); j--) { - Xfree(flist[j]); - if (finfo[j].properties) Xfree((char *) finfo[j].properties); - } - if (flist) Xfree((char *) flist); - if (finfo) Xfree((char *) finfo); - UnlockDisplay(dpy); - SyncHandle(); - return ((char **) NULL); + reply.nameLength = 0; /* avoid trying to read more replies */ + reply_left = 0; + goto badmem; } - if (reply.nameLength == 0) + reply_left = reply.length - + ((SIZEOF(xListFontsWithInfoReply) - SIZEOF(xGenericReply)) >> 2); + if (reply.nameLength == 0) { + _XEatDataWords(dpy, reply_left); break; + } + if (reply.nReplies >= (INT_MAX - i)) /* avoid overflowing size */ + goto badmem; if ((i + reply.nReplies) >= size) { size = i + reply.nReplies + 1; + if (size >= (INT_MAX / sizeof(XFontStruct))) + goto badmem; + if (finfo) { - XFontStruct * tmp_finfo = (XFontStruct *) - Xrealloc ((char *) finfo, - (unsigned) (sizeof(XFontStruct) * size)); - char ** tmp_flist = (char **) - Xrealloc ((char *) flist, - (unsigned) (sizeof(char *) * (size+1))); + XFontStruct * tmp_finfo; + char ** tmp_flist; + tmp_finfo = Xrealloc (finfo, sizeof(XFontStruct) * size); if (tmp_finfo) finfo = tmp_finfo; + else + goto badmem; + + tmp_flist = Xrealloc (flist, sizeof(char *) * (size+1)); if (tmp_flist) flist = tmp_flist; - - if ((! tmp_finfo) || (! tmp_flist)) { - /* free all the memory that we allocated */ - for (j=(i-1); (j >= 0); j--) { - Xfree(flist[j]); - if (finfo[j].properties) - Xfree((char *) finfo[j].properties); - } - Xfree((char *) flist); - Xfree((char *) finfo); - goto clearwire; - } + else + goto badmem; } else { - if (! (finfo = (XFontStruct *) - Xmalloc((unsigned) (sizeof(XFontStruct) * size)))) + if (! (finfo = Xmalloc(sizeof(XFontStruct) * size))) goto clearwire; - if (! (flist = (char **) - Xmalloc((unsigned) (sizeof(char *) * (size+1))))) { + if (! (flist = Xmalloc(sizeof(char *) * (size+1)))) { Xfree((char *) finfo); goto clearwire; } @@ -138,24 +132,27 @@ XFontStruct **info) /* RETURN */ fs->max_bounds = * (XCharStruct *) &reply.maxBounds; fs->n_properties = reply.nFontProps; + fs->properties = NULL; if (fs->n_properties > 0) { - nbytes = reply.nFontProps * sizeof(XFontProp); - if (! (fs->properties = (XFontProp *) Xmalloc((unsigned) nbytes))) - goto badmem; + /* nFontProps is a CARD16 */ nbytes = reply.nFontProps * SIZEOF(xFontProp); + if ((nbytes >> 2) <= reply_left) { + size_t pbytes = reply.nFontProps * sizeof(XFontProp); + fs->properties = Xmalloc (pbytes); + } + if (! fs->properties) + goto badmem; _XRead32 (dpy, (long *)fs->properties, nbytes); + reply_left -= (nbytes >> 2); + } - } else - fs->properties = NULL; - - j = reply.nameLength + 1; + /* nameLength is a CARD8 */ + nbytes = reply.nameLength + 1; if (!i) - j++; /* make first string 1 byte longer, to match XListFonts */ - flist[i] = (char *) Xmalloc ((unsigned int) j); + nbytes++; /* make first string 1 byte longer, to match XListFonts */ + flist[i] = Xmalloc (nbytes); if (! flist[i]) { if (finfo[i].properties) Xfree((char *) finfo[i].properties); - nbytes = (reply.nameLength + 3) & ~3; - _XEatData(dpy, (unsigned long) nbytes); goto badmem; } if (!i) { @@ -185,19 +182,15 @@ XFontStruct **info) /* RETURN */ clearwire: /* Clear the wire. */ - do { - if (reply.nFontProps) - _XEatData(dpy, (unsigned long) - (reply.nFontProps * SIZEOF(xFontProp))); - nbytes = (reply.nameLength + 3) & ~3; - _XEatData(dpy, (unsigned long) nbytes); - } - while (_XReply(dpy,(xReply *) &reply, ((SIZEOF(xListFontsWithInfoReply) - - SIZEOF(xGenericReply)) >> 2), - xFalse) && (reply.nameLength != 0)); - + _XEatDataWords(dpy, reply_left); + while ((reply.nameLength != 0) && + _XReply(dpy, (xReply *) &reply, + ((SIZEOF(xListFontsWithInfoReply) - SIZEOF(xGenericReply)) + >> 2), xTrue)); UnlockDisplay(dpy); SyncHandle(); + *info = NULL; + *actualCount = 0; return (char **) NULL; } -- cgit v1.2.3 From 7d18bbe93809a209dcd3590c4f519f19251323d9 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 22:49:01 -0800 Subject: integer overflow in XGetMotionEvents() [CVE-2013-1981 4/13] If the reported number of motion events is too large, the calculations to allocate memory for them may overflow, leaving us writing beyond the bounds of the allocation. v2: Ensure nEvents is set to 0 when returning NULL events pointer Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetMoEv.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/nx-X11/lib/X11/GetMoEv.c b/nx-X11/lib/X11/GetMoEv.c index 3db176feb..ad9c77277 100644 --- a/nx-X11/lib/X11/GetMoEv.c +++ b/nx-X11/lib/X11/GetMoEv.c @@ -28,6 +28,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include XTimeCoord *XGetMotionEvents( register Display *dpy, @@ -39,7 +40,6 @@ XTimeCoord *XGetMotionEvents( xGetMotionEventsReply rep; register xGetMotionEventsReq *req; XTimeCoord *tc = NULL; - long nbytes; LockDisplay(dpy); GetReq(GetMotionEvents, req); req->window = w; @@ -52,26 +52,22 @@ XTimeCoord *XGetMotionEvents( return (NULL); } - if (rep.nEvents) { - if (! (tc = (XTimeCoord *) - Xmalloc( (unsigned) - (nbytes = (long) rep.nEvents * sizeof(XTimeCoord))))) { - _XEatData (dpy, (unsigned long) nbytes); - UnlockDisplay(dpy); - SyncHandle(); - return (NULL); - } + if (rep.nEvents && (rep.nEvents < (INT_MAX / sizeof(XTimeCoord)))) + tc = Xmalloc(rep.nEvents * sizeof(XTimeCoord)); + if (tc == NULL) { + /* server returned either no events or a bad event count */ + *nEvents = 0; + _XEatDataWords (dpy, rep.length); } - - *nEvents = rep.nEvents; - nbytes = SIZEOF (xTimecoord); + else { register XTimeCoord *tcptr; - register int i; + unsigned int i; xTimecoord xtc; + *nEvents = (int) rep.nEvents; for (i = rep.nEvents, tcptr = tc; i > 0; i--, tcptr++) { - _XRead (dpy, (char *) &xtc, nbytes); + _XRead (dpy, (char *) &xtc, SIZEOF (xTimecoord)); tcptr->time = xtc.time; tcptr->x = cvtINT16toShort (xtc.x); tcptr->y = cvtINT16toShort (xtc.y); -- cgit v1.2.3 From 8673bf0715160943da4937bac25cfeecd3e58b81 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 22:49:01 -0800 Subject: integer overflow in XListHosts() [CVE-2013-1981 5/13] If the reported number of host entries is too large, the calculations to allocate memory for them may overflow, leaving us writing beyond the bounds of the allocation. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LiHosts.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/nx-X11/lib/X11/LiHosts.c b/nx-X11/lib/X11/LiHosts.c index 63ecc508a..6ec956e72 100644 --- a/nx-X11/lib/X11/LiHosts.c +++ b/nx-X11/lib/X11/LiHosts.c @@ -62,6 +62,8 @@ X Window System is a trademark of The Open Group. #include #endif #include "Xlibint.h" +#include + /* * can be freed using XFree. */ @@ -73,7 +75,6 @@ XHostAddress *XListHosts ( { register XHostAddress *outbuf = NULL, *op; xListHostsReply reply; - long nbytes; unsigned char *buf, *bp; register unsigned i; register xListHostsReq *req; @@ -90,19 +91,26 @@ XHostAddress *XListHosts ( } if (reply.nHosts) { - nbytes = reply.length << 2; /* compute number of bytes in reply */ + unsigned long nbytes = reply.length << 2; /* number of bytes in reply */ + const unsigned long max_hosts = INT_MAX / + (sizeof(XHostAddress) + sizeof(XServerInterpretedAddress)); + + if (reply.nHosts < max_hosts) { + unsigned long hostbytes = reply.nHosts * + (sizeof(XHostAddress) + sizeof(XServerInterpretedAddress)); - op = outbuf = (XHostAddress *) - Xmalloc((unsigned) (nbytes + - (reply.nHosts * sizeof(XHostAddress)) + - (reply.nHosts * sizeof(XServerInterpretedAddress)))); + if (reply.length < (INT_MAX >> 2) && + (hostbytes >> 2) < ((INT_MAX >> 2) - reply.length)) + outbuf = Xmalloc(nbytes + hostbytes); + } if (! outbuf) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, reply.length); UnlockDisplay(dpy); SyncHandle(); return (XHostAddress *) NULL; } + op = outbuf; sip = (XServerInterpretedAddress *) (((unsigned char *) outbuf) + (reply.nHosts * sizeof(XHostAddress))); bp = buf = ((unsigned char *) sip) -- cgit v1.2.3 From 0349af1145cb70985bc4cba2d439a7b50d6d95ea Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 11:44:19 -0800 Subject: Integer overflows in stringSectionSize() cause buffer overflow in ReadColornameDB() [CVE-2013-1981 6/13] LoadColornameDB() calls stringSectionSize() to do a first pass over the file (which may be provided by the user via XCMSDB environment variable) to determine how much memory needs to be allocated to read in the file, then allocates the returned sizes and calls ReadColornameDB() to load the data from the file into that newly allocated memory. If stringSectionSize() overflows the signed ints used to calculate the file size (say if you have an xcmsdb with ~4 billion lines in or a combined string length of ~4 gig - which while it may have been inconceivable when Xlib was written, is quite possible today), then LoadColornameDB() may allocate a memory buffer much smaller than the amount of data ReadColornameDB() will write to it. The total size is left limited to an int, because if your xcmsdb file is larger than 2gb, you're doing it wrong. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/cmsColNm.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/nx-X11/lib/X11/cmsColNm.c b/nx-X11/lib/X11/cmsColNm.c index 82ffab5f6..5f9724822 100644 --- a/nx-X11/lib/X11/cmsColNm.c +++ b/nx-X11/lib/X11/cmsColNm.c @@ -40,6 +40,7 @@ #include #include #include +#include #define XK_LATIN1 #include #include "Cv.h" @@ -542,7 +543,10 @@ stringSectionSize( char *pBuf; char *f1; char *f2; - int i; + size_t i; + + unsigned int numEntries = 0; + unsigned int sectionSize = 0; *pNumEntries = 0; *pSectionSize = 0; @@ -576,26 +580,37 @@ stringSectionSize( return(XcmsFailure); } - (*pNumEntries)++; + numEntries++; + if (numEntries >= INT_MAX) + return(XcmsFailure); - (*pSectionSize) += (i = strlen(f1)) + 1; + i = strlen(f1); + if (i >= INT_MAX - sectionSize) + return(XcmsFailure); + sectionSize += i + 1; for (; i; i--, f1++) { /* REMOVE SPACES FROM COUNT */ if (isspace(*f1)) { - (*pSectionSize)--; + sectionSize--; } } - (*pSectionSize) += (i = strlen(f2)) + 1; + i = strlen(f2); + if (i >= INT_MAX - sectionSize) + return(XcmsFailure); + sectionSize += i + 1; for (; i; i--, f2++) { /* REMOVE SPACES FROM COUNT */ if (isspace(*f2)) { - (*pSectionSize)--; + sectionSize--; } } } + *pNumEntries = (int) numEntries; + *pSectionSize = (int) sectionSize; + return(XcmsSuccess); } -- cgit v1.2.3 From 00d7a2e5ba30ded2d3d9ecc696bf324586a380b0 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 18:37:37 -0800 Subject: integer overflow in ReadInFile() in Xrm.c [CVE-2013-1981 7/13] Called from XrmGetFileDatabase() which gets called from InitDefaults() which gets the filename from getenv ("XENVIRONMENT") If file is exactly 0xffffffff bytes long (or longer and truncates to 0xffffffff, on implementations where off_t is larger than an int), then size may be set to a value which overflows causing less memory to be allocated than is written to by the following read() call. size is left limited to an int, because if your Xresources file is larger than 2gb, you're very definitely doing it wrong. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XrmI.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nx-X11/lib/X11/XrmI.h b/nx-X11/lib/X11/XrmI.h index a1615f4e5..63ed257ed 100644 --- a/nx-X11/lib/X11/XrmI.h +++ b/nx-X11/lib/X11/XrmI.h @@ -35,11 +35,13 @@ from The Open Group. #include #include +#include #define GetSizeOfFile(fd,size) \ { \ struct stat status_buffer; \ - if ( (fstat((fd), &status_buffer)) == -1 ) \ + if ( ((fstat((fd), &status_buffer)) == -1 ) || \ + (status_buffer.st_size >= INT_MAX) ) \ size = -1; \ else \ size = status_buffer.st_size; \ -- cgit v1.2.3 From 8468165ae6ec55c715721c6e1991f67902b30564 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 18:37:37 -0800 Subject: integer truncation in _XimParseStringFile() [CVE-2013-1981 8/13] Called from _XimCreateDefaultTree() which uses getenv("XCOMPOSEFILE") to specify filename. If the size of off_t is larger than the size of unsigned long (as in 32-bit builds with large file flags), a file larger than 4 gigs could have its size truncated, leading to data from that file being written past the end of the undersized buffer allocated for it. While configure.ac does not use AC_SYS_LARGEFILE to set large file mode, builders may have added the large file compilation flags to CFLAGS on their own. size is left limited to an int, because if your Xim file is larger than 2gb, you're doing it wrong. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/imLcPrs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nx-X11/lib/X11/imLcPrs.c b/nx-X11/lib/X11/imLcPrs.c index 549fe523a..5b9a2844e 100644 --- a/nx-X11/lib/X11/imLcPrs.c +++ b/nx-X11/lib/X11/imLcPrs.c @@ -41,6 +41,7 @@ OR PERFORMANCE OF THIS SOFTWARE. #include "Ximint.h" #include #include +#include #define XLC_BUFSIZE 256 @@ -674,6 +675,8 @@ _XimParseStringFile( if (fstat (fileno (fp), &st) != -1) { unsigned long size = (unsigned long) st.st_size; + if (st.st_size >= INT_MAX) + return; if (size <= sizeof tb) tbp = tb; else tbp = malloc (size); -- cgit v1.2.3 From 25172302a39c4e0c90dffbdcfa88b99ac442a2f9 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 13:18:48 -0800 Subject: integer overflows in TransFileName() [CVE-2013-1981 9/13] When trying to process file paths the tokens %H, %L, & %S are expanded to $HOME, the standard compose file path & the xlocaledir path. If enough of these tokens are repeated and values like $HOME are set to very large values, the calculation of the total string size required to hold the expanded path can overflow, resulting in allocating a smaller string than the amount of data we'll write to it. Simply restrict all of these values, and the total path size to PATH_MAX, because really, that's all you should need for a filename path. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/imLcPrs.c | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/nx-X11/lib/X11/imLcPrs.c b/nx-X11/lib/X11/imLcPrs.c index 5b9a2844e..f02e93bbb 100644 --- a/nx-X11/lib/X11/imLcPrs.c +++ b/nx-X11/lib/X11/imLcPrs.c @@ -42,6 +42,7 @@ OR PERFORMANCE OF THIS SOFTWARE. #include #include #include +#include "pathmax.h" #define XLC_BUFSIZE 256 @@ -305,9 +306,9 @@ static char* TransFileName(Xim im, char *name) { char *home = NULL, *lcCompose = NULL; - char dir[XLC_BUFSIZE]; - char *i = name, *ret, *j; - int l = 0; + char dir[XLC_BUFSIZE] = ""; + char *i = name, *ret = NULL, *j; + size_t l = 0; while (*i) { if (*i == '%') { @@ -317,30 +318,51 @@ TransFileName(Xim im, char *name) l++; break; case 'H': - home = getenv("HOME"); - if (home) - l += strlen(home); + if (home == NULL) + home = getenv("HOME"); + if (home) { + size_t Hsize = strlen(home); + if (Hsize > PATH_MAX) + /* your home directory length is ridiculous */ + goto end; + l += Hsize; + } break; case 'L': if (lcCompose == NULL) lcCompose = _XlcFileName(im->core.lcd, COMPOSE_FILE); - if (lcCompose) - l += strlen(lcCompose); + if (lcCompose) { + size_t Lsize = strlen(lcCompose); + if (Lsize > PATH_MAX) + /* your compose pathname length is ridiculous */ + goto end; + l += Lsize; + } break; case 'S': - xlocaledir(dir, XLC_BUFSIZE); - l += strlen(dir); + if (dir[0] == '\0') + xlocaledir(dir, XLC_BUFSIZE); + if (dir[0]) { + size_t Ssize = strlen(dir); + if (Ssize > PATH_MAX) + /* your locale directory path length is ridiculous */ + goto end; + l += Ssize; + } break; } } else { l++; } i++; + if (l > PATH_MAX) + /* your expanded path length is ridiculous */ + goto end; } j = ret = Xmalloc(l+1); if (ret == NULL) - return ret; + goto end; i = name; while (*i) { if (*i == '%') { @@ -372,6 +394,7 @@ TransFileName(Xim im, char *name) } } *j = '\0'; +end: Xfree(lcCompose); return ret; } -- cgit v1.2.3 From 361d36770ba3ceef0272e53c59c169f16f16ecf6 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 8 Mar 2013 22:25:35 -0800 Subject: integer overflow in XGetWindowProperty() [CVE-2013-1981 10/13] If the reported number of properties is too large, the calculations to allocate memory for them may overflow, leaving us returning less memory to the caller than implied by the value written to *nitems. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetProp.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/nx-X11/lib/X11/GetProp.c b/nx-X11/lib/X11/GetProp.c index a80c19c50..a9c357d69 100644 --- a/nx-X11/lib/X11/GetProp.c +++ b/nx-X11/lib/X11/GetProp.c @@ -28,6 +28,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include int XGetWindowProperty( @@ -66,8 +67,17 @@ XGetWindowProperty( *prop = (unsigned char *) NULL; if (reply.propertyType != None) { - long nbytes, netbytes; - switch (reply.format) { + unsigned long nbytes, netbytes; + int format = reply.format; + + /* + * Protect against both integer overflow and just plain oversized + * memory allocation - no server should ever return this many props. + */ + if (reply.nItems >= (INT_MAX >> 4)) + format = -1; /* fall through to default error case */ + + switch (format) { /* * One extra byte is malloced than is needed to contain the property * data, but this last byte is null terminated and convenient for @@ -76,24 +86,21 @@ XGetWindowProperty( */ case 8: nbytes = netbytes = reply.nItems; - if (nbytes + 1 > 0 && - (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))) + if (nbytes + 1 > 0 && (*prop = Xmalloc (nbytes + 1))) _XReadPad (dpy, (char *) *prop, netbytes); break; case 16: nbytes = reply.nItems * sizeof (short); netbytes = reply.nItems << 1; - if (nbytes + 1 > 0 && - (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))) + if (nbytes + 1 > 0 && (*prop = Xmalloc (nbytes + 1))) _XRead16Pad (dpy, (short *) *prop, netbytes); break; case 32: nbytes = reply.nItems * sizeof (long); netbytes = reply.nItems << 2; - if (nbytes + 1 > 0 && - (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))) + if (nbytes + 1 > 0 && (*prop = Xmalloc (nbytes + 1))) _XRead32 (dpy, (long *) *prop, netbytes); break; @@ -115,7 +122,7 @@ XGetWindowProperty( break; } if (! *prop) { - _XEatData(dpy, (unsigned long) netbytes); + _XEatDataWords(dpy, reply.length); UnlockDisplay(dpy); SyncHandle(); return(BadAlloc); /* not Success */ -- cgit v1.2.3 From 9501bce22ee691b8de707e6b8ffdbd7e7e71058c Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: integer overflow in XGetImage() [CVE-2013-1981 11/13] Ensure that we don't underallocate when the server claims to have sent a very large reply. Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetImage.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/nx-X11/lib/X11/GetImage.c b/nx-X11/lib/X11/GetImage.c index ddd434a81..59fb45eb1 100644 --- a/nx-X11/lib/X11/GetImage.c +++ b/nx-X11/lib/X11/GetImage.c @@ -30,6 +30,7 @@ in this Software without prior written authorization from The Open Group. #include "Xlibint.h" #include /* for XDestroyImage */ #include "ImUtil.h" +#include #define ROUNDUP(nbytes, pad) (((((nbytes) - 1) + (pad)) / (pad)) * (pad)) @@ -56,7 +57,7 @@ XImage *XGetImage ( xGetImageReply rep; register xGetImageReq *req; char *data; - long nbytes; + unsigned long nbytes; XImage *image; LockDisplay(dpy); GetReq (GetImage, req); @@ -78,10 +79,13 @@ XImage *XGetImage ( return (XImage *)NULL; } - nbytes = (long)rep.length << 2; - data = (char *) Xmalloc((unsigned) nbytes); + if (rep.length < (INT_MAX >> 2)) { + nbytes = (unsigned long)rep.length << 2; + data = Xmalloc(nbytes); + } else + data = NULL; if (! data) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (XImage *) NULL; -- cgit v1.2.3 From 748af521e9e8d82b8f32d5efe73fa7ad3eebaf71 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: integer overflow in XGetPointerMapping() & XGetKeyboardMapping() [CVE-2013-1981 12/13] Ensure that we don't underallocate when the server claims a very large reply Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetPntMap.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/nx-X11/lib/X11/GetPntMap.c b/nx-X11/lib/X11/GetPntMap.c index 0fcdb6696..29fdf21f0 100644 --- a/nx-X11/lib/X11/GetPntMap.c +++ b/nx-X11/lib/X11/GetPntMap.c @@ -29,6 +29,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include #ifdef MIN /* some systems define this in */ #undef MIN @@ -42,7 +43,7 @@ int XGetPointerMapping ( { unsigned char mapping[256]; /* known fixed size */ - long nbytes, remainder = 0; + unsigned long nbytes, remainder = 0; xGetPointerMappingReply rep; register xReq *req; @@ -54,9 +55,15 @@ int XGetPointerMapping ( return 0; } - nbytes = (long)rep.length << 2; - /* Don't count on the server returning a valid value */ + if (rep.length >= (INT_MAX >> 2)) { + _XEatDataWords(dpy, rep.length); + UnlockDisplay(dpy); + SyncHandle(); + return 0; + } + + nbytes = (unsigned long) rep.length << 2; if (nbytes > sizeof mapping) { remainder = nbytes - sizeof mapping; nbytes = sizeof mapping; @@ -69,7 +76,7 @@ int XGetPointerMapping ( } if (remainder) - _XEatData(dpy, (unsigned long)remainder); + _XEatData(dpy, remainder); UnlockDisplay(dpy); SyncHandle(); @@ -86,8 +93,8 @@ XGetKeyboardMapping (Display *dpy, int count, int *keysyms_per_keycode) { - long nbytes; - unsigned long nkeysyms; + unsigned long nbytes; + CARD32 nkeysyms; register KeySym *mapping = NULL; xGetKeyboardMappingReply rep; register xGetKeyboardMappingReq *req; @@ -102,17 +109,19 @@ XGetKeyboardMapping (Display *dpy, return (KeySym *) NULL; } - nkeysyms = (unsigned long) rep.length; + nkeysyms = rep.length; if (nkeysyms > 0) { - nbytes = nkeysyms * sizeof (KeySym); - mapping = (KeySym *) Xmalloc ((unsigned) nbytes); - nbytes = nkeysyms << 2; + if (nkeysyms < (INT_MAX / sizeof (KeySym))) { + nbytes = nkeysyms * sizeof (KeySym); + mapping = Xmalloc (nbytes); + } if (! mapping) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (KeySym *) NULL; } + nbytes = nkeysyms << 2; _XRead32 (dpy, (long *) mapping, nbytes); } *keysyms_per_keycode = rep.keySymsPerKeyCode; -- cgit v1.2.3 From 306ca006a54c5f74a6fe90eb794efa06ff33b259 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: integer overflow in XGetModifierMapping() [CVE-2013-1981 13/13] Ensure that we don't underallocate when the server claims a very large reply Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/ModMap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/nx-X11/lib/X11/ModMap.c b/nx-X11/lib/X11/ModMap.c index c99bfdd5f..122ca80db 100644 --- a/nx-X11/lib/X11/ModMap.c +++ b/nx-X11/lib/X11/ModMap.c @@ -28,6 +28,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include XModifierKeymap * XGetModifierMapping(register Display *dpy) @@ -41,13 +42,17 @@ XGetModifierMapping(register Display *dpy) GetEmptyReq(GetModifierMapping, req); (void) _XReply (dpy, (xReply *)&rep, 0, xFalse); - nbytes = (unsigned long)rep.length << 2; - res = (XModifierKeymap *) Xmalloc(sizeof (XModifierKeymap)); - if (res) res->modifiermap = (KeyCode *) Xmalloc ((unsigned) nbytes); + if (rep.length < (LONG_MAX >> 2)) { + nbytes = (unsigned long)rep.length << 2; + res = Xmalloc(sizeof (XModifierKeymap)); + if (res) + res->modifiermap = Xmalloc (nbytes); + } else + res = NULL; if ((! res) || (! res->modifiermap)) { if (res) Xfree((char *) res); res = (XModifierKeymap *) NULL; - _XEatData(dpy, nbytes); + _XEatDataWords(dpy, rep.length); } else { _XReadPad(dpy, (char *) res->modifiermap, (long) nbytes); res->max_keypermod = rep.numKeyPerModifier; -- cgit v1.2.3 From 05b72b8da17d6077b085608c128ec9e698473bb6 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 16:56:16 -0800 Subject: Convert more _XEatData callers to _XEatDataWords Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetAtomNm.c | 4 ++-- nx-X11/lib/X11/LiICmaps.c | 8 ++++---- nx-X11/lib/X11/LiProps.c | 8 ++++---- nx-X11/lib/X11/OpenDis.c | 2 +- nx-X11/lib/X11/QuColors.c | 10 +++++----- nx-X11/lib/X11/QuTree.c | 8 ++++---- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/nx-X11/lib/X11/GetAtomNm.c b/nx-X11/lib/X11/GetAtomNm.c index 9823c690c..996f7ebf9 100644 --- a/nx-X11/lib/X11/GetAtomNm.c +++ b/nx-X11/lib/X11/GetAtomNm.c @@ -78,7 +78,7 @@ char *XGetAtomName( name[rep.nameLength] = '\0'; _XUpdateAtomCache(dpy, name, atom, 0, -1, 0); } else { - _XEatData(dpy, (unsigned long) (rep.nameLength + 3) & ~3); + _XEatDataWords(dpy, rep.length); name = (char *) NULL; } UnlockDisplay(dpy); @@ -176,7 +176,7 @@ XGetAtomNames ( _XUpdateAtomCache(dpy, names_return[missed], atoms[missed], 0, -1, 0); } else { - _XEatData(dpy, (unsigned long) (rep.nameLength + 3) & ~3); + _XEatDataWords(dpy, rep.length); async_state.status = 0; } } diff --git a/nx-X11/lib/X11/LiICmaps.c b/nx-X11/lib/X11/LiICmaps.c index e98161916..45a2f2fd3 100644 --- a/nx-X11/lib/X11/LiICmaps.c +++ b/nx-X11/lib/X11/LiICmaps.c @@ -34,7 +34,7 @@ Colormap *XListInstalledColormaps( Window win, int *n) /* RETURN */ { - long nbytes; + unsigned long nbytes; Colormap *cmaps; xListInstalledColormapsReply rep; register xResourceReq *req; @@ -51,14 +51,14 @@ Colormap *XListInstalledColormaps( if (rep.nColormaps) { nbytes = rep.nColormaps * sizeof(Colormap); - cmaps = (Colormap *) Xmalloc((unsigned) nbytes); - nbytes = rep.nColormaps << 2; + cmaps = Xmalloc(nbytes); if (! cmaps) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return((Colormap *) NULL); } + nbytes = rep.nColormaps << 2; _XRead32 (dpy, (long *) cmaps, nbytes); } else cmaps = (Colormap *) NULL; diff --git a/nx-X11/lib/X11/LiProps.c b/nx-X11/lib/X11/LiProps.c index 72560aba7..d9c746563 100644 --- a/nx-X11/lib/X11/LiProps.c +++ b/nx-X11/lib/X11/LiProps.c @@ -34,7 +34,7 @@ Atom *XListProperties( Window window, int *n_props) /* RETURN */ { - long nbytes; + unsigned long nbytes; xListPropertiesReply rep; Atom *properties; register xResourceReq *req; @@ -50,14 +50,14 @@ Atom *XListProperties( if (rep.nProperties) { nbytes = rep.nProperties * sizeof(Atom); - properties = (Atom *) Xmalloc ((unsigned) nbytes); - nbytes = rep.nProperties << 2; + properties = Xmalloc (nbytes); if (! properties) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (Atom *) NULL; } + nbytes = rep.nProperties << 2; _XRead32 (dpy, (long *) properties, nbytes); } else properties = (Atom *) NULL; diff --git a/nx-X11/lib/X11/OpenDis.c b/nx-X11/lib/X11/OpenDis.c index a17b8b161..a4b80633b 100644 --- a/nx-X11/lib/X11/OpenDis.c +++ b/nx-X11/lib/X11/OpenDis.c @@ -788,7 +788,7 @@ fallback_success: dpy->xdefaults[reply.nItems] = '\0'; } else if (reply.propertyType != None) - _XEatData(dpy, reply.nItems * (reply.format >> 3)); + _XEatDataWords(dpy, reply.length); } #if !USE_XCB DeqAsyncHandler(dpy, &async); diff --git a/nx-X11/lib/X11/QuColors.c b/nx-X11/lib/X11/QuColors.c index e51375fa0..a8bc8837b 100644 --- a/nx-X11/lib/X11/QuColors.c +++ b/nx-X11/lib/X11/QuColors.c @@ -37,9 +37,7 @@ XQueryColors( int ncolors) { register int i; - xrgb *color; xQueryColorsReply rep; - long nbytes; register xQueryColorsReq *req; LockDisplay(dpy); @@ -53,8 +51,9 @@ XQueryColors( /* XXX this isn't very efficient */ if (_XReply(dpy, (xReply *) &rep, 0, xFalse) != 0) { - if ((color = (xrgb *) - Xmalloc((unsigned) (nbytes = (long) ncolors * SIZEOF(xrgb))))) { + unsigned long nbytes = (long) ncolors * SIZEOF(xrgb); + xrgb *color = Xmalloc(nbytes); + if (color != NULL) { _XRead(dpy, (char *) color, nbytes); @@ -68,7 +67,8 @@ XQueryColors( } Xfree((char *)color); } - else _XEatData(dpy, (unsigned long) nbytes); + else + _XEatDataWords(dpy, rep.length); } UnlockDisplay(dpy); SyncHandle(); diff --git a/nx-X11/lib/X11/QuTree.c b/nx-X11/lib/X11/QuTree.c index 3cea282fa..8da2ae261 100644 --- a/nx-X11/lib/X11/QuTree.c +++ b/nx-X11/lib/X11/QuTree.c @@ -37,7 +37,7 @@ Status XQueryTree ( Window **children, /* RETURN */ unsigned int *nchildren) /* RETURN */ { - long nbytes; + unsigned long nbytes; xQueryTreeReply rep; register xResourceReq *req; @@ -52,14 +52,14 @@ Status XQueryTree ( *children = (Window *) NULL; if (rep.nChildren != 0) { nbytes = rep.nChildren * sizeof(Window); - *children = (Window *) Xmalloc((unsigned) nbytes); - nbytes = rep.nChildren << 2; + *children = Xmalloc(nbytes); if (! *children) { - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (0); } + nbytes = rep.nChildren << 2; _XRead32 (dpy, (long *) *children, nbytes); } *parent = rep.parent; -- cgit v1.2.3 From b20710586d97c69c47aa657c29c8df661f6fe417 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 13:03:55 -0800 Subject: Move repeated #ifdef magic to find PATH_MAX into a common header Lets stop duplicating the mess all over Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetDflt.c | 25 +-------------- nx-X11/lib/X11/Imakefile | 2 +- nx-X11/lib/X11/lcFile.c | 24 +------------- nx-X11/lib/X11/pathmax.h | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 48 deletions(-) create mode 100644 nx-X11/lib/X11/pathmax.h diff --git a/nx-X11/lib/X11/GetDflt.c b/nx-X11/lib/X11/GetDflt.c index 0c00b3c94..749893c7e 100644 --- a/nx-X11/lib/X11/GetDflt.c +++ b/nx-X11/lib/X11/GetDflt.c @@ -52,30 +52,7 @@ SOFTWARE. #include "Xlibint.h" #include #include - -#ifndef X_NOT_POSIX -#ifdef _POSIX_SOURCE -#include -#else -#define _POSIX_SOURCE -#include -#undef _POSIX_SOURCE -#endif -#endif -#ifndef PATH_MAX -#ifdef WIN32 -#define PATH_MAX 512 -#else -#include -#endif -#ifndef PATH_MAX -#ifdef MAXPATHLEN -#define PATH_MAX MAXPATHLEN -#else -#define PATH_MAX 1024 -#endif -#endif -#endif +#include "pathmax.h" #ifdef XTHREADS #include diff --git a/nx-X11/lib/X11/Imakefile b/nx-X11/lib/X11/Imakefile index d4045b615..5a1fc5ff2 100644 --- a/nx-X11/lib/X11/Imakefile +++ b/nx-X11/lib/X11/Imakefile @@ -1146,7 +1146,7 @@ MakeLintSubdirs($(LINTSUBDIRS),install.ln,install.ln) #endif #endif -includes:: XlibConf.h +includes:: XlibConf.h pathmax.h #include diff --git a/nx-X11/lib/X11/lcFile.c b/nx-X11/lib/X11/lcFile.c index 50159f64b..8c2e7c062 100644 --- a/nx-X11/lib/X11/lcFile.c +++ b/nx-X11/lib/X11/lcFile.c @@ -54,29 +54,7 @@ #define XLC_BUFSIZE 256 -#ifndef X_NOT_POSIX -#ifdef _POSIX_SOURCE -#include -#else -#define _POSIX_SOURCE -#include -#undef _POSIX_SOURCE -#endif -#endif -#ifndef PATH_MAX -#ifdef WIN32 -#define PATH_MAX 512 -#else -#include -#endif -#ifndef PATH_MAX -#ifdef MAXPATHLEN -#define PATH_MAX MAXPATHLEN -#else -#define PATH_MAX 1024 -#endif -#endif -#endif +#include "pathmax.h" #define NUM_LOCALEDIR 64 diff --git a/nx-X11/lib/X11/pathmax.h b/nx-X11/lib/X11/pathmax.h new file mode 100644 index 000000000..ffec295e0 --- /dev/null +++ b/nx-X11/lib/X11/pathmax.h @@ -0,0 +1,82 @@ + +/*********************************************************** + +Copyright 1987, 1988, 1998 The Open Group + +Permission to use, copy, modify, distribute, and sell this software and its +documentation for any purpose is hereby granted without fee, provided that +the above copyright notice appear in all copies and that both that +copyright notice and this permission notice appear in supporting +documentation. + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +OPEN GROUP BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN +AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +Except as contained in this notice, the name of The Open Group shall not be +used in advertising or otherwise to promote the sale, use or other dealings +in this Software without prior written authorization from The Open Group. + + +Copyright 1987, 1988 by Digital Equipment Corporation, Maynard, Massachusetts. + + All Rights Reserved + +Permission to use, copy, modify, and distribute this software and its +documentation for any purpose and without fee is hereby granted, +provided that the above copyright notice appear in all copies and that +both that copyright notice and this permission notice appear in +supporting documentation, and that the name of Digital not be +used in advertising or publicity pertaining to distribution of the +software without specific, written prior permission. + +DIGITAL DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE, INCLUDING +ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO EVENT SHALL +DIGITAL BE LIABLE FOR ANY SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR +ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, +WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS +SOFTWARE. + +******************************************************************/ + +/* + * Provides a single definition of PATH_MAX instead of replicating this mess + * in multiple files + */ + +#ifdef HAVE_CONFIG_H +#include +#endif +#include + +#ifndef X_NOT_POSIX +#ifdef _POSIX_SOURCE +#include +#else +#define _POSIX_SOURCE +#include +#undef _POSIX_SOURCE +#endif +#endif +#ifndef PATH_MAX +#ifdef WIN32 +#define PATH_MAX 512 +#else +#include +#endif +#ifndef PATH_MAX +#ifdef MAXPATHLEN +#define PATH_MAX MAXPATHLEN +#else +#define PATH_MAX 1024 +#endif +#endif +#endif + -- cgit v1.2.3 From e03f3922a3d97174916273e369b2e9a90f53a05a Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 9 Mar 2013 11:04:37 -0800 Subject: Make XGetWindowProperty() always initialize returned values Avoids memory corruption and other errors when callers access them without checking to see if XGetWindowProperty() returned an error value. Callers are still required to check for errors, this just reduces the damage when they don't. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetProp.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/nx-X11/lib/X11/GetProp.c b/nx-X11/lib/X11/GetProp.c index a9c357d69..4149199f5 100644 --- a/nx-X11/lib/X11/GetProp.c +++ b/nx-X11/lib/X11/GetProp.c @@ -49,6 +49,13 @@ XGetWindowProperty( register xGetPropertyReq *req; xError error; + /* Always initialize return values, in case callers fail to initialize + them and fail to check the return code for an error. */ + *actual_type = None; + *actual_format = 0; + *nitems = *bytesafter = 0L; + *prop = (unsigned char *) NULL; + LockDisplay(dpy); GetReq (GetProperty, req); req->window = w; @@ -65,7 +72,6 @@ XGetWindowProperty( return (1); /* not Success */ } - *prop = (unsigned char *) NULL; if (reply.propertyType != None) { unsigned long nbytes, netbytes; int format = reply.format; -- cgit v1.2.3 From 2a1fbb1818bc56342e23e026004e0e7c4059cb6a Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 19:30:09 -0800 Subject: unvalidated lengths in XAllocColorCells() [CVE-2013-1997 1/15] If a broken server returned larger than requested values for nPixels or nMasks, XAllocColorCells would happily overflow the buffers provided by the caller to write the results into. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/AllCells.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/AllCells.c b/nx-X11/lib/X11/AllCells.c index bbc977059..6d9548f0d 100644 --- a/nx-X11/lib/X11/AllCells.c +++ b/nx-X11/lib/X11/AllCells.c @@ -54,8 +54,13 @@ Status XAllocColorCells( status = _XReply(dpy, (xReply *)&rep, 0, xFalse); if (status) { - _XRead32 (dpy, (long *) pixels, 4L * (long) (rep.nPixels)); - _XRead32 (dpy, (long *) masks, 4L * (long) (rep.nMasks)); + if ((rep.nPixels > ncolors) || (rep.nMasks > nplanes)) { + _XEatDataWords(dpy, rep.length); + status = 0; /* Failure */ + } else { + _XRead32 (dpy, (long *) pixels, 4L * (long) (rep.nPixels)); + _XRead32 (dpy, (long *) masks, 4L * (long) (rep.nMasks)); + } } UnlockDisplay(dpy); -- cgit v1.2.3 From b06952603552dacb59a9808d4042bc18c48842bd Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 1 Mar 2013 22:49:01 -0800 Subject: unvalidated index in _XkbReadGetDeviceInfoReply() [CVE-2013-1997 2/15] If the X server returns more buttons than are allocated in the XKB device info structures, out of bounds writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBExtDev.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nx-X11/lib/X11/XKBExtDev.c b/nx-X11/lib/X11/XKBExtDev.c index de5570337..47b94a3aa 100644 --- a/nx-X11/lib/X11/XKBExtDev.c +++ b/nx-X11/lib/X11/XKBExtDev.c @@ -181,6 +181,9 @@ int tmp; return tmp; } if (rep->nBtnsWanted>0) { + if (((unsigned short) rep->firstBtnWanted + rep->nBtnsWanted) + >= devi->num_btns) + goto BAILOUT; act= &devi->btn_acts[rep->firstBtnWanted]; bzero((char *)act,(rep->nBtnsWanted*sizeof(XkbAction))); } @@ -190,6 +193,9 @@ int tmp; goto BAILOUT; if (rep->nBtnsRtrn>0) { int size; + if (((unsigned short) rep->firstBtnRtrn + rep->nBtnsRtrn) + >= devi->num_btns) + goto BAILOUT; act= &devi->btn_acts[rep->firstBtnRtrn]; size= rep->nBtnsRtrn*SIZEOF(xkbActionWireDesc); if (!_XkbCopyFromReadBuffer(&buf,(char *)act,size)) -- cgit v1.2.3 From 0445730b1396dd21c4908373599c7197c0bfc5a6 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 09:12:47 -0800 Subject: unvalidated indexes in _XkbReadGeomShapes() [CVE-2013-1997 3/15] If the X server returns shape indexes outside the range of the number of shapes it told us to allocate, out of bounds memory access could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGeom.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/nx-X11/lib/X11/XKBGeom.c b/nx-X11/lib/X11/XKBGeom.c index 5ab51b682..b145e7eda 100644 --- a/nx-X11/lib/X11/XKBGeom.c +++ b/nx-X11/lib/X11/XKBGeom.c @@ -364,12 +364,16 @@ Status rtrn; } ol->num_points= olWire->nPoints; } - if (shapeWire->primaryNdx!=XkbNoShape) + if ((shapeWire->primaryNdx!=XkbNoShape) && + (shapeWire->primaryNdx < shapeWire->nOutlines)) shape->primary= &shape->outlines[shapeWire->primaryNdx]; - else shape->primary= NULL; - if (shapeWire->approxNdx!=XkbNoShape) + else + shape->primary= NULL; + if ((shapeWire->approxNdx!=XkbNoShape) && + (shapeWire->approxNdx < shapeWire->nOutlines)) shape->approx= &shape->outlines[shapeWire->approxNdx]; - else shape->approx= NULL; + else + shape->approx= NULL; XkbComputeShapeBounds(shape); } return Success; -- cgit v1.2.3 From e6fbdea84a23ab88ff1ec98ba179273cab09adfb Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 09:18:26 -0800 Subject: unvalidated indexes in _XkbReadGetGeometryReply() [CVE-2013-1997 4/15] If the X server returns color indexes outside the range of the number of colors it told us to allocate, out of bounds memory access could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGeom.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nx-X11/lib/X11/XKBGeom.c b/nx-X11/lib/X11/XKBGeom.c index b145e7eda..1f9628bea 100644 --- a/nx-X11/lib/X11/XKBGeom.c +++ b/nx-X11/lib/X11/XKBGeom.c @@ -619,6 +619,9 @@ XkbGeometryPtr geom; if (status==Success) status= _XkbReadGeomKeyAliases(&buf,geom,rep); left= _XkbFreeReadBuffer(&buf); + if ((rep->baseColorNdx > geom->num_colors) || + (rep->labelColorNdx > geom->num_colors)) + status = BadLength; if ((status!=Success) || left || buf.error) { if (status==Success) status= BadLength; -- cgit v1.2.3 From 5dae1d3f4bcbc1a5c3869e04bdbdd0c7198bd3c7 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 09:28:33 -0800 Subject: unvalidated index in _XkbReadKeySyms() [CVE-2013-1997 5/15] If the X server returns keymap indexes outside the range of the number of keys it told us to allocate, out of bounds memory access could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index a38907671..440cc818d 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -152,9 +152,12 @@ XkbClientMapPtr map; map= xkb->map; if (map->key_sym_map==NULL) { register int offset; + int size = xkb->max_key_code + 1; XkbSymMapPtr oldMap; xkbSymMapWireDesc *newMap; - map->key_sym_map= _XkbTypedCalloc((xkb->max_key_code+1),XkbSymMapRec); + if (((unsigned short)rep->firstKeySym + rep->nKeySyms) > size) + return BadLength; + map->key_sym_map= _XkbTypedCalloc(size,XkbSymMapRec); if (map->key_sym_map==NULL) return BadAlloc; if (map->syms==NULL) { @@ -210,6 +213,8 @@ XkbClientMapPtr map; KeySym * newSyms; int tmp; + if (((unsigned short)rep->firstKeySym + rep->nKeySyms) > map->num_syms) + return BadLength; oldMap = &map->key_sym_map[rep->firstKeySym]; for (i=0;i<(int)rep->nKeySyms;i++,oldMap++) { newMap= (xkbSymMapWireDesc *) -- cgit v1.2.3 From 4c19cd0c73c769d70125e08e6351bc98134bb058 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 09:40:22 -0800 Subject: unvalidated index in _XkbReadKeyActions() [CVE-2013-1997 6/15] If the X server returns key action indexes outside the range of the number of keys it told us to allocate, out of bounds memory access could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 440cc818d..8f99625b0 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -270,6 +270,10 @@ Status ret = Success; symMap = &info->map->key_sym_map[rep->firstKeyAct]; for (i=0;i<(int)rep->nKeyActs;i++,symMap++) { if (numDesc[i]==0) { + if ((i + rep->firstKeyAct) > (info->max_key_code + 1)) { + ret = BadLength; + goto done; + } info->server->key_acts[i+rep->firstKeyAct]= 0; } else { -- cgit v1.2.3 From 8f2c050828c3f8b2574f720625ee6ee7041f3d36 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 10:39:21 -0800 Subject: unvalidated index in _XkbReadKeyBehaviors() [CVE-2013-1997 7/15] If the X server returns key behavior indexes outside the range of the number of keys it told us to allocate, out of bounds memory writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 8f99625b0..79fa315cd 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -306,8 +306,10 @@ register int i; xkbBehaviorWireDesc *wire; if ( rep->totalKeyBehaviors>0 ) { + int size = xkb->max_key_code + 1; + if ( ((int) rep->firstKeyBehavior + rep->nKeyBehaviors) > size) + return BadLength; if ( xkb->server->behaviors == NULL ) { - int size = xkb->max_key_code+1; xkb->server->behaviors = _XkbTypedCalloc(size,XkbBehavior); if (xkb->server->behaviors==NULL) return BadAlloc; @@ -319,7 +321,7 @@ xkbBehaviorWireDesc *wire; for (i=0;itotalKeyBehaviors;i++) { wire= (xkbBehaviorWireDesc *)_XkbGetReadBufferPtr(buf, SIZEOF(xkbBehaviorWireDesc)); - if (wire==NULL) + if (wire==NULL || wire->key >= size) return BadLength; xkb->server->behaviors[wire->key].type= wire->type; xkb->server->behaviors[wire->key].data= wire->data; -- cgit v1.2.3 From 7564bf7efc93b87da7a512a0ba205c9d1d26ff36 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 10:51:51 -0800 Subject: unvalidated index in _XkbReadModifierMap() [CVE-2013-1997 8/15] If the X server returns modifier map indexes outside the range of the number of keys it told us to allocate, out of bounds memory writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 79fa315cd..56f068dbf 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -391,6 +391,9 @@ register int i; unsigned char *wire; if ( rep->totalModMapKeys>0 ) { + if ( ((int)rep->firstModMapKey + rep->nModMapKeys) > + (xkb->max_key_code + 1)) + return BadLength; if ((xkb->map->modmap==NULL)&& (XkbAllocClientMap(xkb,XkbModifierMapMask,0)!=Success)) { return BadAlloc; @@ -403,6 +406,8 @@ unsigned char *wire; if (!wire) return BadLength; for (i=0;itotalModMapKeys;i++,wire+=2) { + if (wire[0] > xkb->max_key_code || wire[1] > xkb->max_key_code) + return BadLength; xkb->map->modmap[wire[0]]= wire[1]; } } -- cgit v1.2.3 From e27df80700727725aea6c03986fa10e4ed0c6d65 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 11:04:44 -0800 Subject: unvalidated index in _XkbReadExplicitComponents() [CVE-2013-1997 9/15] If the X server returns key indexes outside the range of the number of keys it told us to allocate, out of bounds memory writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 56f068dbf..5705f2649 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -363,8 +363,10 @@ register int i; unsigned char *wire; if ( rep->totalKeyExplicit>0 ) { + int size = xkb->max_key_code + 1; + if ( ((int) rep->firstKeyExplicit + rep->nKeyExplicit) > size) + return BadLength; if ( xkb->server->explicit == NULL ) { - int size = xkb->max_key_code+1; xkb->server->explicit = _XkbTypedCalloc(size,unsigned char); if (xkb->server->explicit==NULL) return BadAlloc; @@ -378,6 +380,8 @@ unsigned char *wire; if (!wire) return BadLength; for (i=0;itotalKeyExplicit;i++,wire+=2) { + if (wire[0] > xkb->max_key_code || wire[1] > xkb->max_key_code) + return BadLength; xkb->server->explicit[wire[0]]= wire[1]; } } -- cgit v1.2.3 From e6d8856ef465f9ab973026236df15d975e9cb069 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 11:01:04 -0800 Subject: unvalidated index in _XkbReadVirtualModMap() [CVE-2013-1997 10/15] If the X server returns modifier map indexes outside the range of the number of keys it told us to allocate, out of bounds memory writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 5705f2649..ea2e6b512 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -426,6 +426,9 @@ xkbVModMapWireDesc * wire; XkbServerMapPtr srv; if ( rep->totalVModMapKeys>0 ) { + if (((int) rep->firstVModMapKey + rep->nVModMapKeys) + > xkb->max_key_code) + return BadLength; if (((xkb->server==NULL)||(xkb->server->vmodmap==NULL))&& (XkbAllocServerMap(xkb,XkbVirtualModMapMask,0)!=Success)) { return BadAlloc; -- cgit v1.2.3 From 0bf09b4bb1e469f37b75a088b1a5ba093ab36252 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 11:11:08 -0800 Subject: unvalidated index/length in _XkbReadGetNamesReply() [CVE-2013-1997 11/15] If the X server returns key name indexes outside the range of the number of keys it told us to allocate, out of bounds memory writes could occur. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBNames.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nx-X11/lib/X11/XKBNames.c b/nx-X11/lib/X11/XKBNames.c index 04cd6f3e3..1f5207493 100644 --- a/nx-X11/lib/X11/XKBNames.c +++ b/nx-X11/lib/X11/XKBNames.c @@ -180,6 +180,8 @@ _XkbReadGetNamesReply( Display * dpy, nKeys= xkb->max_key_code+1; names->keys= _XkbTypedCalloc(nKeys,XkbKeyNameRec); } + else if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code) + goto BAILOUT; if (names->keys!=NULL) { if (!_XkbCopyFromReadBuffer(&buf, (char *)&names->keys[rep->firstKey], -- cgit v1.2.3 From 0284afb80cefe1ae3c2567dd46427b5d425791b1 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 11:25:25 -0800 Subject: unvalidated length in _XimXGetReadData() [CVE-2013-1997 12/15] Check the provided buffer size against the amount of data we're going to write into it, not against the reported length from the ClientMessage. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/imTrX.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nx-X11/lib/X11/imTrX.c b/nx-X11/lib/X11/imTrX.c index eba328057..2b5455f1b 100644 --- a/nx-X11/lib/X11/imTrX.c +++ b/nx-X11/lib/X11/imTrX.c @@ -372,7 +372,7 @@ _XimXGetReadData( XFree(prop_ret); return False; } - if (buf_len >= length) { + if (buf_len >= (int)nitems) { (void)memcpy(buf, prop_ret, (int)nitems); *ret_len = (int)nitems; if (bytes_after_ret > 0) { -- cgit v1.2.3 From f6c5069ac78d2fe9883cc7ddaf1f32cc17d27107 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: Avoid overflows in XListFonts() [CVE-2013-1997 13/15] Ensure that when breaking the returned list into individual strings, we don't walk past the end of allocated memory to write the '\0' bytes Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/FontNames.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/nx-X11/lib/X11/FontNames.c b/nx-X11/lib/X11/FontNames.c index 3018cf2cf..b5bc7b4ba 100644 --- a/nx-X11/lib/X11/FontNames.c +++ b/nx-X11/lib/X11/FontNames.c @@ -29,6 +29,7 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include char ** XListFonts( @@ -40,11 +41,13 @@ int *actualCount) /* RETURN */ register long nbytes; register unsigned i; register int length; - char **flist; - char *ch; + char **flist = NULL; + char *ch = NULL; + char *chend; + int count = 0; xListFontsReply rep; register xListFontsReq *req; - register long rlen; + unsigned long rlen; LockDisplay(dpy); GetReq(ListFonts, req); @@ -62,15 +65,17 @@ int *actualCount) /* RETURN */ } if (rep.nFonts) { - flist = (char **)Xmalloc ((unsigned)rep.nFonts * sizeof(char *)); - rlen = rep.length << 2; - ch = (char *) Xmalloc((unsigned) (rlen + 1)); + flist = Xmalloc (rep.nFonts * sizeof(char *)); + if (rep.length < (LONG_MAX >> 2)) { + rlen = rep.length << 2; + ch = Xmalloc(rlen + 1); /* +1 to leave room for last null-terminator */ + } if ((! flist) || (! ch)) { if (flist) Xfree((char *) flist); if (ch) Xfree(ch); - _XEatData(dpy, (unsigned long) rlen); + _XEatDataWords(dpy, rep.length); *actualCount = 0; UnlockDisplay(dpy); SyncHandle(); @@ -81,17 +86,21 @@ int *actualCount) /* RETURN */ /* * unpack into null terminated strings. */ + chend = ch + (rlen + 1); length = *(unsigned char *)ch; *ch = 1; /* make sure it is non-zero for XFreeFontNames */ for (i = 0; i < rep.nFonts; i++) { - flist[i] = ch + 1; /* skip over length */ - ch += length + 1; /* find next length ... */ - length = *(unsigned char *)ch; - *ch = '\0'; /* and replace with null-termination */ + if (ch + length < chend) { + flist[i] = ch + 1; /* skip over length */ + ch += length + 1; /* find next length ... */ + length = *(unsigned char *)ch; + *ch = '\0'; /* and replace with null-termination */ + count++; + } else + flist[i] = NULL; } } - else flist = (char **) NULL; - *actualCount = rep.nFonts; + *actualCount = count; UnlockDisplay(dpy); SyncHandle(); return (flist); -- cgit v1.2.3 From 77edd88e104cbd3fb5dc95473adac1db2c2302a3 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: Avoid overflows in XGetFontPath() [CVE-2013-1997 14/15] Ensure that when breaking the returned list into individual strings, we don't walk past the end of allocated memory to write the '\0' bytes Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/GetFPath.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/nx-X11/lib/X11/GetFPath.c b/nx-X11/lib/X11/GetFPath.c index 7d497c92e..abd4a5dbd 100644 --- a/nx-X11/lib/X11/GetFPath.c +++ b/nx-X11/lib/X11/GetFPath.c @@ -28,15 +28,18 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include char **XGetFontPath( register Display *dpy, int *npaths) /* RETURN */ { xGetFontPathReply rep; - register long nbytes; - char **flist; - char *ch; + unsigned long nbytes; + char **flist = NULL; + char *ch = NULL; + char *chend; + int count = 0; register unsigned i; register int length; register xReq *req; @@ -46,16 +49,17 @@ char **XGetFontPath( (void) _XReply (dpy, (xReply *) &rep, 0, xFalse); if (rep.nPaths) { - flist = (char **) - Xmalloc((unsigned) rep.nPaths * sizeof (char *)); - nbytes = (long)rep.length << 2; - ch = (char *) Xmalloc ((unsigned) (nbytes + 1)); + flist = Xmalloc(rep.nPaths * sizeof (char *)); + if (rep.length < (LONG_MAX >> 2)) { + nbytes = (unsigned long) rep.length << 2; + ch = Xmalloc (nbytes + 1); /* +1 to leave room for last null-terminator */ + } if ((! flist) || (! ch)) { if (flist) Xfree((char *) flist); if (ch) Xfree(ch); - _XEatData(dpy, (unsigned long) nbytes); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (char **) NULL; @@ -65,16 +69,20 @@ char **XGetFontPath( /* * unpack into null terminated strings. */ + chend = ch + (nbytes + 1); length = *ch; for (i = 0; i < rep.nPaths; i++) { - flist[i] = ch+1; /* skip over length */ - ch += length + 1; /* find next length ... */ - length = *ch; - *ch = '\0'; /* and replace with null-termination */ + if (ch + length < chend) { + flist[i] = ch+1; /* skip over length */ + ch += length + 1; /* find next length ... */ + length = *ch; + *ch = '\0'; /* and replace with null-termination */ + count++; + } else + flist[i] = NULL; } } - else flist = NULL; - *npaths = rep.nPaths; + *npaths = count; UnlockDisplay(dpy); SyncHandle(); return (flist); -- cgit v1.2.3 From dbc11719399ce7e191c806ad6b5c9104666e2a77 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 15:08:21 -0800 Subject: Avoid overflows in XListExtensions() [CVE-2013-1997 15/15] Ensure that when breaking the returned list into individual strings, we don't walk past the end of allocated memory to write the '\0' bytes Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/ListExt.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/nx-X11/lib/X11/ListExt.c b/nx-X11/lib/X11/ListExt.c index 16b522e88..e925c4773 100644 --- a/nx-X11/lib/X11/ListExt.c +++ b/nx-X11/lib/X11/ListExt.c @@ -28,18 +28,21 @@ in this Software without prior written authorization from The Open Group. #include #endif #include "Xlibint.h" +#include char **XListExtensions( register Display *dpy, int *nextensions) /* RETURN */ { xListExtensionsReply rep; - char **list; - char *ch; + char **list = NULL; + char *ch = NULL; + char *chend; + int count = 0; register unsigned i; register int length; register xReq *req; - register long rlen; + unsigned long rlen; LockDisplay(dpy); GetEmptyReq (ListExtensions, req); @@ -51,16 +54,17 @@ char **XListExtensions( } if (rep.nExtensions) { - list = (char **) Xmalloc ( - (unsigned)(rep.nExtensions * sizeof (char *))); - rlen = rep.length << 2; - ch = (char *) Xmalloc ((unsigned) rlen + 1); + list = Xmalloc (rep.nExtensions * sizeof (char *)); + if (rep.length < (LONG_MAX >> 2)) { + rlen = rep.length << 2; + ch = Xmalloc (rlen + 1); /* +1 to leave room for last null-terminator */ + } if ((!list) || (!ch)) { if (list) Xfree((char *) list); if (ch) Xfree((char *) ch); - _XEatData(dpy, (unsigned long) rlen); + _XEatDataWords(dpy, rep.length); UnlockDisplay(dpy); SyncHandle(); return (char **) NULL; @@ -70,17 +74,21 @@ char **XListExtensions( /* * unpack into null terminated strings. */ + chend = ch + (rlen + 1); length = *ch; for (i = 0; i < rep.nExtensions; i++) { - list[i] = ch+1; /* skip over length */ - ch += length + 1; /* find next length ... */ - length = *ch; - *ch = '\0'; /* and replace with null-termination */ + if (ch + length < chend) { + list[i] = ch+1; /* skip over length */ + ch += length + 1; /* find next length ... */ + length = *ch; + *ch = '\0'; /* and replace with null-termination */ + count++; + } else + list[i] = NULL; } } - else list = (char **) NULL; - *nextensions = rep.nExtensions; + *nextensions = count; UnlockDisplay(dpy); SyncHandle(); return (list); -- cgit v1.2.3 From bddfee4a987c0ef5eb26e1b14b8385e7630a1e21 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 12:01:39 -0800 Subject: Unbounded recursion in GetDatabase() when parsing include files [CVE-2013-2004 1/2] GetIncludeFile() can call GetDatabase() which can call GetIncludeFile() which can call GetDatabase() which can call GetIncludeFile() .... eventually causing recursive stack overflow and crash. Easily reproduced with a resource file that #includes itself. Limit is set to a include depth of 100 files, which should be enough for all known use cases, but could be adjusted later if necessary. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/Xrm.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/nx-X11/lib/X11/Xrm.c b/nx-X11/lib/X11/Xrm.c index b3d92bb48..818e72906 100644 --- a/nx-X11/lib/X11/Xrm.c +++ b/nx-X11/lib/X11/Xrm.c @@ -1087,13 +1087,15 @@ static void GetIncludeFile( XrmDatabase db, _Xconst char *base, _Xconst char *fname, - int fnamelen); + int fnamelen, + int depth); static void GetDatabase( XrmDatabase db, _Xconst register char *str, _Xconst char *filename, - Bool doall) + Bool doall, + int depth) { char *rhs; char *lhs, lhs_s[DEF_BUFF_SIZE]; @@ -1203,7 +1205,8 @@ static void GetDatabase( } while (c != '"' && !is_EOL(bits)); /* must have an ending " */ if (c == '"') - GetIncludeFile(db, filename, fname, str - len - fname); + GetIncludeFile(db, filename, fname, str - len - fname, + depth); } } /* spin to next newline */ @@ -1544,7 +1547,7 @@ XrmPutLineResource( { if (!*pdb) *pdb = NewDatabase(); _XLockMutex(&(*pdb)->linfo); - GetDatabase(*pdb, line, (char *)NULL, False); + GetDatabase(*pdb, line, (char *)NULL, False, 0); _XUnlockMutex(&(*pdb)->linfo); } @@ -1556,7 +1559,7 @@ XrmGetStringDatabase( db = NewDatabase(); _XLockMutex(&db->linfo); - GetDatabase(db, data, (char *)NULL, True); + GetDatabase(db, data, (char *)NULL, True, 0); _XUnlockMutex(&db->linfo); return db; } @@ -1633,7 +1636,8 @@ GetIncludeFile( XrmDatabase db, _Xconst char *base, _Xconst char *fname, - int fnamelen) + int fnamelen, + int depth) { int len; char *str; @@ -1641,6 +1645,8 @@ GetIncludeFile( if (fnamelen <= 0 || fnamelen >= BUFSIZ) return; + if (depth >= MAXDBDEPTH) + return; if (*fname != '/' && base && (str = strrchr(base, '/'))) { len = str - base + 1; if (len + fnamelen >= BUFSIZ) @@ -1654,7 +1660,7 @@ GetIncludeFile( } if (!(str = ReadInFile(realfname))) return; - GetDatabase(db, str, realfname, True); + GetDatabase(db, str, realfname, True, depth + 1); Xfree(str); } @@ -1670,7 +1676,7 @@ XrmGetFileDatabase( db = NewDatabase(); _XLockMutex(&db->linfo); - GetDatabase(db, str, filename, True); + GetDatabase(db, str, filename, True, 0); _XUnlockMutex(&db->linfo); Xfree(str); return db; @@ -1694,7 +1700,7 @@ XrmCombineFileDatabase( } else db = NewDatabase(); _XLockMutex(&db->linfo); - GetDatabase(db, str, filename, True); + GetDatabase(db, str, filename, True, 0); _XUnlockMutex(&db->linfo); Xfree(str); if (!override) -- cgit v1.2.3 From e386187e91569eae3064927d5b5753a7a68ace47 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 12:39:58 -0800 Subject: Unbounded recursion in _XimParseStringFile() when parsing include files [CVE-2013-2004 2/2] parseline() can call _XimParseStringFile() which can call parseline() which can call _XimParseStringFile() which can call parseline() .... eventually causing recursive stack overflow and crash. Limit is set to a include depth of 100 files, which should be enough for all known use cases, but could be adjusted later if necessary. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/imLcPrs.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/nx-X11/lib/X11/imLcPrs.c b/nx-X11/lib/X11/imLcPrs.c index f02e93bbb..ad65da694 100644 --- a/nx-X11/lib/X11/imLcPrs.c +++ b/nx-X11/lib/X11/imLcPrs.c @@ -58,6 +58,8 @@ extern int _Xmbstoutf8( int len ); +static void parsestringfile(FILE *fp, Xim im, int depth); + /* * Parsing File Format: * @@ -447,7 +449,8 @@ static int parseline( FILE *fp, Xim im, - char* tokenbuf) + char* tokenbuf, + int depth) { int token; DTModifier modifier_mask; @@ -494,11 +497,13 @@ parseline( goto error; if ((filename = TransFileName(im, tokenbuf)) == NULL) goto error; + if (++depth > 100) + goto error; infp = _XFopenFile(filename, "r"); Xfree(filename); if (infp == NULL) goto error; - _XimParseStringFile(infp, im); + parsestringfile(infp, im, depth); fclose(infp); return (0); } else if ((token == KEY) && (strcmp("None", tokenbuf) == 0)) { @@ -691,6 +696,15 @@ void _XimParseStringFile( FILE *fp, Xim im) +{ + parsestringfile(fp, im, 0); +} + +static void +parsestringfile( + FILE *fp, + Xim im, + int depth) { char tb[8192]; char* tbp; @@ -704,7 +718,7 @@ _XimParseStringFile( else tbp = malloc (size); if (tbp != NULL) { - while (parseline(fp, im, tbp) >= 0) {} + while (parseline(fp, im, tbp, depth) >= 0) {} if (tbp != tb) free (tbp); } } -- cgit v1.2.3 From 37f8d3eb8ec4a44bebaac7893e5881bd59b5440c Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 16 Mar 2013 10:03:13 -0700 Subject: Use calloc in XOpenDisplay to initialize structs containing pointers Prevents trying to free uninitialized pointers if we have to bail out partway through setup, such as if we receive a corrupted or incomplete connection setup block from the server. Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/OpenDis.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/nx-X11/lib/X11/OpenDis.c b/nx-X11/lib/X11/OpenDis.c index a4b80633b..4036572a8 100644 --- a/nx-X11/lib/X11/OpenDis.c +++ b/nx-X11/lib/X11/OpenDis.c @@ -549,9 +549,7 @@ fallback_success: /* * Now iterate down setup information..... */ - dpy->pixmap_format = - (ScreenFormat *)Xmalloc( - (unsigned) (dpy->nformats *sizeof(ScreenFormat))); + dpy->pixmap_format = Xcalloc(dpy->nformats, sizeof(ScreenFormat)); if (dpy->pixmap_format == NULL) { OutOfMemory (dpy, setup); return(NULL); @@ -579,8 +577,7 @@ fallback_success: /* * next the Screen structures. */ - dpy->screens = - (Screen *)Xmalloc((unsigned) dpy->nscreens*sizeof(Screen)); + dpy->screens = Xcalloc(dpy->nscreens, sizeof(Screen)); if (dpy->screens == NULL) { OutOfMemory (dpy, setup); return(NULL); @@ -622,8 +619,7 @@ fallback_success: /* * lets set up the depth structures. */ - sp->depths = (Depth *)Xmalloc( - (unsigned)sp->ndepths*sizeof(Depth)); + sp->depths = Xcalloc(sp->ndepths, sizeof(Depth)); if (sp->depths == NULL) { OutOfMemory (dpy, setup); return(NULL); @@ -645,8 +641,7 @@ fallback_success: dp->nvisuals = u.dp->nVisuals; u.dp = (xDepth *) (((char *) u.dp) + sz_xDepth); if (dp->nvisuals > 0) { - dp->visuals = - (Visual *)Xmalloc((unsigned)dp->nvisuals*sizeof(Visual)); + dp->visuals = Xcalloc(dp->nvisuals, sizeof(Visual)); if (dp->visuals == NULL) { OutOfMemory (dpy, setup); return(NULL); -- cgit v1.2.3 From dc749a457d62b330051e9fb709960951e05de41b Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sun, 31 Mar 2013 12:22:35 -0700 Subject: _XkbReadGetMapReply: reject maxKeyCodes smaller than the minKeyCode Various other bounds checks in the code assume this is true, so enforce it when we first get the data from the X server. Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/XKBGetMap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index ea2e6b512..5eb1d41bf 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -485,6 +485,8 @@ unsigned mask; if ( xkb->device_spec == XkbUseCoreKbd ) xkb->device_spec= rep->deviceID; + if ( rep->maxKeyCode < rep->minKeyCode ) + return BadImplementation; xkb->min_key_code = rep->minKeyCode; xkb->max_key_code = rep->maxKeyCode; -- cgit v1.2.3 From 838108c296cb739350456f49431b57821c78b15c Mon Sep 17 00:00:00 2001 From: Matthieu Herrb Date: Wed, 8 May 2013 19:33:09 +0200 Subject: XListFontsWithInfo: Re-decrement flist[0] before calling free() on it. Freeing a pointer that wasn't returned by malloc() is undefined behavior and produces an error with OpenBSD's implementation. Signed-off-by: Matthieu Herrb Reviewed-by: Alan Coopersmith Signed-off-by: Alan Coopersmith Signed-off-by: Julien Cristau Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/FontInfo.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/FontInfo.c b/nx-X11/lib/X11/FontInfo.c index 4b295a5eb..0cb5b1910 100644 --- a/nx-X11/lib/X11/FontInfo.c +++ b/nx-X11/lib/X11/FontInfo.c @@ -174,8 +174,10 @@ XFontStruct **info) /* RETURN */ badmem: /* Free all memory allocated by this function. */ for (j=(i-1); (j >= 0); j--) { - Xfree(flist[j]); - if (finfo[j].properties) Xfree((char *) finfo[j].properties); + if (j == 0) + flist[j]--; /* was incremented above */ + Xfree(flist[j]); + if (finfo[j].properties) Xfree((char *) finfo[j].properties); } if (flist) Xfree((char *) flist); if (finfo) Xfree((char *) finfo); -- cgit v1.2.3 From a9f623f0a63372ca0705e8394fadf514dec55b1c Mon Sep 17 00:00:00 2001 From: Julien Cristau Date: Tue, 21 May 2013 21:54:55 +0200 Subject: Add a couple fixups for the security patches Add a couple fixups for the security patches - off-by-one in xkb - memory leak in an error path Backport from debian to NX: Ulrich Sibiller --- nx-X11/lib/X11/Font.c | 1 + nx-X11/lib/X11/XKBGetMap.c | 2 +- nx-X11/lib/X11/XKBNames.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nx-X11/lib/X11/Font.c b/nx-X11/lib/X11/Font.c index 0501ae279..d311cd0f7 100644 --- a/nx-X11/lib/X11/Font.c +++ b/nx-X11/lib/X11/Font.c @@ -508,6 +508,7 @@ _XF86BigfontQueryFont ( any real font needs, so the combined total doesn't overflow either */ if (reply.nUniqCharInfos > ((ULONG_MAX / 2) / SIZEOF(xCharInfo)) || reply.nCharInfos > ((ULONG_MAX / 2) / sizeof(CARD16))) { + Xfree(fs->properties); Xfree((char *) fs); _XEatDataWords(dpy, reply_left); return (XFontStruct *)NULL; diff --git a/nx-X11/lib/X11/XKBGetMap.c b/nx-X11/lib/X11/XKBGetMap.c index 5eb1d41bf..391d7aa89 100644 --- a/nx-X11/lib/X11/XKBGetMap.c +++ b/nx-X11/lib/X11/XKBGetMap.c @@ -427,7 +427,7 @@ XkbServerMapPtr srv; if ( rep->totalVModMapKeys>0 ) { if (((int) rep->firstVModMapKey + rep->nVModMapKeys) - > xkb->max_key_code) + > xkb->max_key_code + 1) return BadLength; if (((xkb->server==NULL)||(xkb->server->vmodmap==NULL))&& (XkbAllocServerMap(xkb,XkbVirtualModMapMask,0)!=Success)) { diff --git a/nx-X11/lib/X11/XKBNames.c b/nx-X11/lib/X11/XKBNames.c index 1f5207493..c82b59e30 100644 --- a/nx-X11/lib/X11/XKBNames.c +++ b/nx-X11/lib/X11/XKBNames.c @@ -180,7 +180,7 @@ _XkbReadGetNamesReply( Display * dpy, nKeys= xkb->max_key_code+1; names->keys= _XkbTypedCalloc(nKeys,XkbKeyNameRec); } - else if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code) + else if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code + 1) goto BAILOUT; if (names->keys!=NULL) { if (!_XkbCopyFromReadBuffer(&buf, -- cgit v1.2.3