debian/changelog | 10 ++ debian/patches/001_hide_xeatdatawords.diff | 33 +++++++ debian/patches/002_security_fixups.diff | 43 ++++++++++ debian/patches/series | 2 include/X11/Xlibint.h | 18 ++++ modules/im/ximcp/Makefile.am | 1 modules/im/ximcp/imLcPrs.c | 73 +++++++++++++---- modules/im/ximcp/imTrX.c | 2 src/AllCells.c | 9 +- src/Font.c | 77 +++++++++++------- src/FontInfo.c | 120 ++++++++++++++--------------- src/FontNames.c | 35 +++++--- src/GetAtomNm.c | 4 src/GetDflt.c | 25 ------ src/GetFPath.c | 36 +++++--- src/GetImage.c | 12 +- src/GetMoEv.c | 26 ++---- src/GetPntMap.c | 31 ++++--- src/GetProp.c | 33 +++++-- src/LiHosts.c | 22 +++-- src/LiICmaps.c | 8 - src/LiProps.c | 8 - src/ListExt.c | 36 +++++--- src/Makefile.am | 1 src/ModMap.c | 13 ++- src/OpenDis.c | 15 +-- src/QuColors.c | 10 +- src/QuTree.c | 8 - src/Xrm.c | 30 +++++-- src/XrmI.h | 4 src/pathmax.h | 82 +++++++++++++++++++ src/xcb_io.c | 17 ++++ src/xcms/cmsColNm.c | 27 +++++- src/xkb/XKBExtDev.c | 6 + src/xkb/XKBGeom.c | 15 ++- src/xkb/XKBGetMap.c | 33 +++++++ src/xkb/XKBNames.c | 2 src/xlibi18n/lcFile.c | 24 ----- 38 files changed, 648 insertions(+), 303 deletions(-)
New commits: commit f8ca86bc2f8747604591ab9cdb41b5f4e57ec6e5 Author: Julien Cristau <[email protected]> Date: Tue May 14 00:23:04 2013 +0200 Upload to squeeze-security diff --git a/debian/changelog b/debian/changelog index b564e18..9edd437 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,13 @@ +libx11 (2:1.3.3-4+squeeze1) squeeze-security; urgency=high + + * CVE-2013-1981: integer overflows calculating memory needs for replies + * CVE-2013-1997: buffer overflows due to not validating length or offset + values in replies + * CVE-2013-2004: unbounded recursion parsing user-specified files + (closes: #145048) + + -- Julien Cristau <[email protected]> Tue, 21 May 2013 22:26:20 +0200 + libx11 (2:1.3.3-4) unstable; urgency=low * Cherry-pick patches from upstream, 1.3-branch: commit 3066d9b286bda9c606776f482953b70dfc389aac Author: Julien Cristau <[email protected]> Date: Tue May 21 21:54:55 2013 +0200 Add a couple fixups for the security patches - off-by-one in xkb - memory leak in an error path diff --git a/debian/patches/002_security_fixups.diff b/debian/patches/002_security_fixups.diff new file mode 100644 index 0000000..cbd8b89 --- /dev/null +++ b/debian/patches/002_security_fixups.diff @@ -0,0 +1,43 @@ +From: Julien Cristau <[email protected]> +Subject: Followup fixes for integer/buffer overflows + +Fix memory leak in _XF86BigfontQueryFont error paths + +Index: libx11/src/Font.c +=================================================================== +--- libx11.orig/src/Font.c ++++ libx11/src/Font.c +@@ -550,6 +550,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; +Index: libx11/src/xkb/XKBGetMap.c +=================================================================== +--- libx11.orig/src/xkb/XKBGetMap.c ++++ libx11/src/xkb/XKBGetMap.c +@@ -429,7 +429,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)) { +Index: libx11/src/xkb/XKBNames.c +=================================================================== +--- libx11.orig/src/xkb/XKBNames.c ++++ libx11/src/xkb/XKBNames.c +@@ -182,7 +182,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) ++ if ( ((int)rep->firstKey + rep->nKeys) > xkb->max_key_code + 1) + goto BAILOUT; + if (names->keys!=NULL) { + if (!_XkbCopyFromReadBuffer(&buf, diff --git a/debian/patches/series b/debian/patches/series index 23c072d..7b1ae6f 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,4 +1,5 @@ 001_hide_xeatdatawords.diff +002_security_fixups.diff 003_recognize_glibc_2.3.2_locale_names.diff 006_tailor_pt_BR.UTF-8_Compose.diff 007_iso8859-15_Compose_fix.diff commit ac4b4c05fc32b900ad23e9be9b954bdf764c4515 Author: Julien Cristau <[email protected]> Date: Tue May 14 01:26:25 2013 +0200 Hide _XEatDataWords diff --git a/debian/patches/001_hide_xeatdatawords.diff b/debian/patches/001_hide_xeatdatawords.diff new file mode 100644 index 0000000..ed55f87 --- /dev/null +++ b/debian/patches/001_hide_xeatdatawords.diff @@ -0,0 +1,33 @@ +Index: libx11/include/X11/Xlibint.h +=================================================================== +--- libx11.orig/include/X11/Xlibint.h ++++ libx11/include/X11/Xlibint.h +@@ -926,10 +926,12 @@ extern void _XEatData( + Display* /* dpy */, + unsigned long /* n */ + ) _XLIB_COLD; ++#ifdef XLIB_WANT_XEATDATAWORDS + extern void _XEatDataWords( + Display* /* dpy */, + unsigned long /* n */ +-) _XLIB_COLD; ++) _XLIB_COLD _X_HIDDEN; ++#endif + #if defined(__SUNPRO_C) /* Studio compiler alternative to "cold" attribute */ + # pragma rarely_called(_XEatData, _XEatDataWords) + #endif +Index: libx11/configure.ac +=================================================================== +--- libx11.orig/configure.ac ++++ libx11/configure.ac +@@ -90,6 +90,10 @@ no) + esac + AC_SUBST(X11_EXTRA_DEPS) + ++# Debian change: _XEatDataWords is not declared by default, to not affect other ++# X libs ++AC_DEFINE(XLIB_WANT_XEATDATAWORDS, 1, [Declare _XEatDataWords in Xlibint.h]) ++ + dnl Issue an error if xtrans.m4 was not found and XTRANS_CONNECTION_FLAGS macro + dnl was not expanded, since libX11 with no transport types is rather useless. + dnl diff --git a/debian/patches/series b/debian/patches/series index f20e030..23c072d 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,3 +1,4 @@ +001_hide_xeatdatawords.diff 003_recognize_glibc_2.3.2_locale_names.diff 006_tailor_pt_BR.UTF-8_Compose.diff 007_iso8859-15_Compose_fix.diff commit 79b5a82ddf969dfee372531b09a705f017f75ae9 Author: Matthieu Herrb <[email protected]> Date: Wed May 8 19:33:09 2013 +0200 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 <[email protected]> Reviewed-by: Alan Coopersmith <[email protected]> Signed-off-by: Alan Coopersmith <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/FontInfo.c b/src/FontInfo.c index a617c12..1770846 100644 --- a/src/FontInfo.c +++ b/src/FontInfo.c @@ -197,8 +197,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); commit 7f37e53ae78f276af98910036e193884e1796a1e Author: Alan Coopersmith <[email protected]> Date: Fri Apr 19 14:30:40 2013 -0700 Give GNU & Solaris Studio compilers hints about XEatData branches Try to offset the cost of all the recent checks we've added by giving the compiler a hint that the branches that involve us eating data are less likely to be used than the ones that process it. Signed-off-by: Alan Coopersmith <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/include/X11/Xlibint.h b/include/X11/Xlibint.h index 27ff89b..72c1132 100644 --- a/include/X11/Xlibint.h +++ b/include/X11/Xlibint.h @@ -894,6 +894,15 @@ typedef struct _XExten { /* private to extension mechanism */ struct _XExten *next_flush; /* next in list of those with flushes */ } _XExtension; +/* Temporary definition until we can depend on an xproto release with it */ +#ifdef _X_COLD +# define _XLIB_COLD _X_COLD +#elif defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 403) /* 4.3+ */ +# define _XLIB_COLD __attribute__((__cold__)) +#else +# define _XLIB_COLD /* nothing */ +#endif + /* extension hooks */ #ifdef DataRoutineIsProcedure @@ -916,11 +925,14 @@ extern int (*_XErrorFunction)( extern void _XEatData( Display* /* dpy */, unsigned long /* n */ -); +) _XLIB_COLD; extern void _XEatDataWords( Display* /* dpy */, unsigned long /* n */ -); +) _XLIB_COLD; +#if defined(__SUNPRO_C) /* Studio compiler alternative to "cold" attribute */ +# pragma rarely_called(_XEatData, _XEatDataWords) +#endif extern char *_XAllocScratch( Display* /* dpy */, unsigned long /* nbytes */ commit 50dc897d518d13378272e28051ed32b5f010363d Author: Alan Coopersmith <[email protected]> Date: Sun Mar 31 12:22:35 2013 -0700 _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 <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/xkb/XKBGetMap.c b/src/xkb/XKBGetMap.c index d06d628..2ecb17e 100644 --- a/src/xkb/XKBGetMap.c +++ b/src/xkb/XKBGetMap.c @@ -487,6 +487,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; commit 2aa36e935be89494bebd26e15a40df35886ba9eb Author: Alan Coopersmith <[email protected]> Date: Sat Mar 16 10:03:13 2013 -0700 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 <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/OpenDis.c b/src/OpenDis.c index ce059be..5e26b3c 100644 --- a/src/OpenDis.c +++ b/src/OpenDis.c @@ -486,9 +486,7 @@ XOpenDisplay ( /* * 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); @@ -516,8 +514,7 @@ XOpenDisplay ( /* * 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); @@ -559,8 +556,7 @@ XOpenDisplay ( /* * 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); @@ -582,8 +578,7 @@ XOpenDisplay ( 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); commit 56b5e82f6058dcf4aeb171a8f23af21e4d4d27ca Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 16:56:16 2013 -0800 Convert more _XEatData callers to _XEatDataWords Signed-off-by: Alan Coopersmith <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/GetAtomNm.c b/src/GetAtomNm.c index 169486b..757a5c7 100644 --- a/src/GetAtomNm.c +++ b/src/GetAtomNm.c @@ -79,7 +79,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); @@ -177,7 +177,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/src/LiICmaps.c b/src/LiICmaps.c index d2cb221..7f2336b 100644 --- a/src/LiICmaps.c +++ b/src/LiICmaps.c @@ -35,7 +35,7 @@ Colormap *XListInstalledColormaps( Window win, int *n) /* RETURN */ { - long nbytes; + unsigned long nbytes; Colormap *cmaps; xListInstalledColormapsReply rep; register xResourceReq *req; @@ -52,14 +52,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/src/LiProps.c b/src/LiProps.c index 344f990..4b4cf3a 100644 --- a/src/LiProps.c +++ b/src/LiProps.c @@ -35,7 +35,7 @@ Atom *XListProperties( Window window, int *n_props) /* RETURN */ { - long nbytes; + unsigned long nbytes; xListPropertiesReply rep; Atom *properties; register xResourceReq *req; @@ -51,14 +51,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/src/OpenDis.c b/src/OpenDis.c index 46e1026..ce059be 100644 --- a/src/OpenDis.c +++ b/src/OpenDis.c @@ -725,7 +725,7 @@ XOpenDisplay ( 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/src/QuColors.c b/src/QuColors.c index 0df6f02..585ae2e 100644 --- a/src/QuColors.c +++ b/src/QuColors.c @@ -38,9 +38,7 @@ XQueryColors( int ncolors) { register int i; - xrgb *color; xQueryColorsReply rep; - long nbytes; register xQueryColorsReq *req; LockDisplay(dpy); @@ -54,8 +52,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); @@ -69,7 +68,8 @@ XQueryColors( } Xfree((char *)color); } - else _XEatData(dpy, (unsigned long) nbytes); + else + _XEatDataWords(dpy, rep.length); } UnlockDisplay(dpy); SyncHandle(); diff --git a/src/QuTree.c b/src/QuTree.c index c40d081..47e6fe2 100644 --- a/src/QuTree.c +++ b/src/QuTree.c @@ -38,7 +38,7 @@ Status XQueryTree ( Window **children, /* RETURN */ unsigned int *nchildren) /* RETURN */ { - long nbytes; + unsigned long nbytes; xQueryTreeReply rep; register xResourceReq *req; @@ -53,14 +53,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; commit ee881d01a550c839e6381f7454048da396463124 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 9 11:04:37 2013 -0800 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 <[email protected]> Signed-off-by: Alan Coopersmith <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/GetProp.c b/src/GetProp.c index 082805e..8a49f13 100644 --- a/src/GetProp.c +++ b/src/GetProp.c @@ -50,6 +50,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; @@ -66,7 +73,6 @@ XGetWindowProperty( return (1); /* not Success */ } - *prop = (unsigned char *) NULL; if (reply.propertyType != None) { unsigned long nbytes, netbytes; int format = reply.format; commit 8fb6da68cc3ceee00e8fc8872e728a2d1384d5d7 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 15:08:21 2013 -0800 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 <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/ListExt.c b/src/ListExt.c index 5a41ec3..567f2d9 100644 --- a/src/ListExt.c +++ b/src/ListExt.c @@ -29,18 +29,21 @@ in this Software without prior written authorization from The Open Group. #include <config.h> #endif #include "Xlibint.h" +#include <limits.h> 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); @@ -52,16 +55,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; @@ -71,17 +75,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); commit b674808eb182420852733f6d27f83c0c9ed1bcd3 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 15:08:21 2013 -0800 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 <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/GetFPath.c b/src/GetFPath.c index 484d162..d6af165 100644 --- a/src/GetFPath.c +++ b/src/GetFPath.c @@ -29,15 +29,18 @@ in this Software without prior written authorization from The Open Group. #include <config.h> #endif #include "Xlibint.h" +#include <limits.h> 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; @@ -47,16 +50,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; @@ -66,16 +70,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); commit 1bb1354adadfea468def96c2c4ec5242f4796886 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 15:08:21 2013 -0800 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 <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/FontNames.c b/src/FontNames.c index c073592..4c0a3dd 100644 --- a/src/FontNames.c +++ b/src/FontNames.c @@ -30,6 +30,7 @@ in this Software without prior written authorization from The Open Group. #include <config.h> #endif #include "Xlibint.h" +#include <limits.h> char ** XListFonts( @@ -41,11 +42,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); @@ -63,15 +66,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(); @@ -82,17 +87,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); commit e2ca81487bf31dbffe826c6f800d2f0d9019dcc5 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 15:08:21 2013 -0800 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 <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/ModMap.c b/src/ModMap.c index 28c2da5..bdf524d 100644 --- a/src/ModMap.c +++ b/src/ModMap.c @@ -29,6 +29,7 @@ in this Software without prior written authorization from The Open Group. #include <config.h> #endif #include "Xlibint.h" +#include <limits.h> XModifierKeymap * XGetModifierMapping(register Display *dpy) @@ -42,13 +43,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; commit 5a0ab3331b1125bae13d6a5c26799f15c4141845 Author: Alan Coopersmith <[email protected]> Date: Sat Mar 2 15:08:21 2013 -0800 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 <[email protected]> Reviewed-by: Matthieu Herrb <[email protected]> Signed-off-by: Julien Cristau <[email protected]> diff --git a/src/GetPntMap.c b/src/GetPntMap.c index 5f65ffe..afc0402 100644 --- a/src/GetPntMap.c +++ b/src/GetPntMap.c @@ -30,6 +30,7 @@ in this Software without prior written authorization from The Open Group. #include <config.h> #endif #include "Xlibint.h" +#include <limits.h> #ifdef MIN /* some systems define this in <sys/param.h> */ #undef MIN @@ -43,7 +44,7 @@ int XGetPointerMapping ( { unsigned char mapping[256]; /* known fixed size */ - long nbytes, remainder = 0; + unsigned long nbytes, remainder = 0; xGetPointerMappingReply rep; register xReq *req; @@ -55,9 +56,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; @@ -70,7 +77,7 @@ int XGetPointerMapping ( } if (remainder) - _XEatData(dpy, (unsigned long)remainder); + _XEatData(dpy, remainder); UnlockDisplay(dpy); SyncHandle(); @@ -87,8 +94,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; @@ -103,17 +110,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; commit c9e456c893281603d1cfcffb925fe30b9d3c322e Author: Alan Coopersmith <[email protected]> -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
