From 8f82f69e9e4b76b01b9f5fe06d1ea86851d951c4 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Wed, 22 Jan 2014 22:37:15 -0800 Subject: [PATCH 04/31] dix: integer overflow in RegionSizeof() [CVE-2014-8092 3/4] RegionSizeof contains several integer overflows if a large length value is passed in. Once we fix it to return 0 on overflow, we also have to fix the callers to handle this error condition v2: Fixed limit calculation in RegionSizeof as pointed out by jcristau. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Peter Hutterer Reviewed-by: Julien Cristau Signed-off-by: Fedora X Ninjas --- dix/region.c | 20 +++++++++++++------- include/regionstr.h | 10 +++++++--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/dix/region.c b/dix/region.c index 737d2a8..76c4e29 100644 --- a/dix/region.c +++ b/dix/region.c @@ -169,7 +169,6 @@ Equipment Corporation. ((r1)->y1 <= (r2)->y1) && \ ((r1)->y2 >= (r2)->y2) ) -#define xallocData(n) malloc(RegionSizeof(n)) #define xfreeData(reg) if ((reg)->data && (reg)->data->size) free((reg)->data) #define RECTALLOC_BAIL(pReg,n,bail) \ @@ -205,8 +204,9 @@ if (!(pReg)->data || (((pReg)->data->numRects + (n)) > (pReg)->data->size)) \ #define DOWNSIZE(reg,numRects) \ if (((numRects) < ((reg)->data->size >> 1)) && ((reg)->data->size > 50)) \ { \ - RegDataPtr NewData; \ - NewData = (RegDataPtr)realloc((reg)->data, RegionSizeof(numRects)); \ + size_t NewSize = RegionSizeof(numRects); \ + RegDataPtr NewData = \ + (NewSize > 0) ? realloc((reg)->data, NewSize) : NULL ; \ if (NewData) \ { \ NewData->size = (numRects); \ @@ -330,17 +330,20 @@ Bool RegionRectAlloc(RegionPtr pRgn, int n) { RegDataPtr data; + size_t rgnSize; if (!pRgn->data) { n++; - pRgn->data = xallocData(n); + rgnSize = RegionSizeof(n); + pRgn->data = (rgnSize > 0) ? malloc(rgnSize) : NULL; if (!pRgn->data) return RegionBreak(pRgn); pRgn->data->numRects = 1; *RegionBoxptr(pRgn) = pRgn->extents; } else if (!pRgn->data->size) { - pRgn->data = xallocData(n); + rgnSize = RegionSizeof(n); + pRgn->data = (rgnSize > 0) ? malloc(rgnSize) : NULL; if (!pRgn->data) return RegionBreak(pRgn); pRgn->data->numRects = 0; @@ -352,7 +355,8 @@ RegionRectAlloc(RegionPtr pRgn, int n) n = 250; } n += pRgn->data->numRects; - data = (RegDataPtr) realloc(pRgn->data, RegionSizeof(n)); + rgnSize = RegionSizeof(n); + data = (rgnSize > 0) ? realloc(pRgn->data, rgnSize) : NULL; if (!data) return RegionBreak(pRgn); pRgn->data = data; @@ -1297,6 +1301,7 @@ RegionFromRects(int nrects, xRectangle *prect, int ctype) { RegionPtr pRgn; + size_t rgnSize; RegDataPtr pData; BoxPtr pBox; int i; @@ -1323,7 +1328,8 @@ RegionFromRects(int nrects, xRectangle *prect, int ctype) } return pRgn; } - pData = xallocData(nrects); + rgnSize = RegionSizeof(nrects); + pData = (rgnSize > 0) ? malloc(rgnSize) : NULL; if (!pData) { RegionBreak(pRgn); return pRgn; diff --git a/include/regionstr.h b/include/regionstr.h index 805257b..5006207 100644 --- a/include/regionstr.h +++ b/include/regionstr.h @@ -127,7 +127,10 @@ RegionEnd(RegionPtr reg) static inline size_t RegionSizeof(int n) { - return (sizeof(RegDataRec) + ((n) * sizeof(BoxRec))); + if (n < ((INT_MAX - sizeof(RegDataRec)) / sizeof(BoxRec))) + return (sizeof(RegDataRec) + ((n) * sizeof(BoxRec))); + else + return 0; } static inline void @@ -138,9 +141,10 @@ RegionInit(RegionPtr _pReg, BoxPtr _rect, int _size) (_pReg)->data = (RegDataPtr) NULL; } else { + size_t rgnSize; (_pReg)->extents = RegionEmptyBox; - if (((_size) > 1) && ((_pReg)->data = - (RegDataPtr) malloc(RegionSizeof(_size)))) { + if (((_size) > 1) && ((rgnSize = RegionSizeof(_size)) > 0) && + (((_pReg)->data = malloc(rgnSize)) != NULL)) { (_pReg)->data->size = (_size); (_pReg)->data->numRects = 0; } -- 1.9.3