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(-) (limited to 'nx-X11/lib/X11/Font.c') 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(-) (limited to 'nx-X11/lib/X11/Font.c') 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 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 + 1 file changed, 1 insertion(+) (limited to 'nx-X11/lib/X11/Font.c') 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; -- cgit v1.2.3