From cc90f6be64bfd6973ae270b9bff494f577e1bda7 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 26 Apr 2013 23:59:25 -0700 Subject: [PATCH 4/5] integer overflows in XpGetPrinterList() [CVE-2013-2062 2/3] listCount is a CARD32 that needs to be bounds checked before it is multiplied by the size of the structs to allocate, and the string lengths are CARD32s and need to be bounds checked before adding one to them to come up with the total size to allocate, to avoid integer overflow leading to underallocation and writing data from the network past the end of the allocated buffer. Signed-off-by: Alan Coopersmith --- src/XpPrinter.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/XpPrinter.c b/src/XpPrinter.c index bdc96e6..03b18c4 100644 --- a/src/XpPrinter.c +++ b/src/XpPrinter.c @@ -42,6 +42,7 @@ #include #include #include "XpExtUtil.h" +#include #define _XpPadOut(len) (((len) + 3) & ~3) @@ -62,7 +63,7 @@ XpGetPrinterList ( long dataLenVR; CARD8 *dataVR; /* aka STRING8 */ - XPPrinterList ptr_list; + XPPrinterList ptr_list = NULL; XExtDisplayInfo *info = (XExtDisplayInfo *) xp_find_display (dpy); @@ -128,13 +129,12 @@ XpGetPrinterList ( *list_count = rep.listCount; if (*list_count) { - ptr_list = (XPPrinterList) - Xmalloc( (unsigned) (sizeof(XPPrinterRec) * (*list_count + 1))); + if (rep.listCount < (INT_MAX / sizeof(XPPrinterRec))) + ptr_list = Xmalloc(sizeof(XPPrinterRec) * (*list_count + 1)); if (!ptr_list) { - UnlockDisplay(dpy); - SyncHandle(); - return ( (XPPrinterList) NULL ); /* malloc error */ + _XEatDataWords(dpy, rep.length); + goto out; } /* @@ -150,16 +150,17 @@ XpGetPrinterList ( _XRead32 (dpy, &dataLenVR, (long) sizeof(CARD32) ); if (dataLenVR) { - dataVR = (CARD8 *) Xmalloc( (unsigned) dataLenVR + 1 ); + if (dataLenVR < INT_MAX) + dataVR = Xmalloc(dataLenVR + 1); + else + dataVR = NULL; if (!dataVR) { - UnlockDisplay(dpy); - SyncHandle(); - return ( (XPPrinterList) NULL ); /* malloc error */ + _XEatData(dpy, dataLenVR); + } else { + _XReadPad (dpy, (char *) dataVR, (long) dataLenVR); + dataVR[dataLenVR] = 0; } - - _XReadPad (dpy, (char *) dataVR, (long) dataLenVR); - dataVR[dataLenVR] = 0; ptr_list[i].name = (char *) dataVR; } else { @@ -172,16 +173,17 @@ XpGetPrinterList ( _XRead32 (dpy, &dataLenVR, (long) sizeof(CARD32) ); if (dataLenVR) { - dataVR = (CARD8 *) Xmalloc( (unsigned) dataLenVR + 1 ); + if (dataLenVR < INT_MAX) + dataVR = Xmalloc(dataLenVR + 1); + else + dataVR = NULL; if (!dataVR) { - UnlockDisplay(dpy); - SyncHandle(); - return ( (XPPrinterList) NULL ); /* malloc error */ + _XEatData(dpy, dataLenVR); + } else { + _XReadPad (dpy, (char *) dataVR, (long) dataLenVR); + dataVR[dataLenVR] = 0; } - - _XReadPad (dpy, (char *) dataVR, (long) dataLenVR); - dataVR[dataLenVR] = 0; ptr_list[i].desc = (char *) dataVR; } else { @@ -193,6 +195,7 @@ XpGetPrinterList ( ptr_list = (XPPrinterList) NULL; } + out: UnlockDisplay(dpy); SyncHandle(); -- 1.8.2.3