From 07e0eba7dbf3522e749da28fb55e3921d72d9b6b Mon Sep 17 00:00:00 2001 From: Hardening Date: Thu, 29 May 2014 10:12:02 +0200 Subject: [PATCH] Check that bpp has reasonable value As bpp is often used for malloc computations, let's check that it has a reasonable value. --- client/X11/xf_cliprdr.c | 11 +++++++++-- libfreerdp/core/orders.c | 21 +++++++++++++++++++++ libfreerdp/core/surface.c | 6 ++++++ libfreerdp/core/window.c | 23 +++++++++++++++-------- 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 19c433279..8fb49f98e 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -914,7 +914,7 @@ static void xf_cliprdr_process_unicodetext(clipboardContext* cb, BYTE* data, int crlf2lf(cb->data, &cb->data_length); } -static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size) +static BOOL xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size) { wStream* s; UINT16 bpp; @@ -926,12 +926,18 @@ static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size) if (size < 40) { DEBUG_X11_CLIPRDR("dib size %d too short", size); - return; + return FALSE; } s = Stream_New(data, size); Stream_Seek(s, 14); Stream_Read_UINT16(s, bpp); + if ((bpp < 1) || (bpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, bpp); + return FALSE; + } + Stream_Read_UINT32(s, ncolors); offset = 14 + 40 + (bpp <= 8 ? (ncolors == 0 ? (1 << bpp) : ncolors) * 4 : 0); Stream_Free(s, FALSE); @@ -949,6 +955,7 @@ static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size) cb->data = Stream_Buffer(s); cb->data_length = Stream_GetPosition(s); Stream_Free(s, FALSE); + return TRUE; } static void xf_cliprdr_process_html(clipboardContext* cb, BYTE* data, int size) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 6a3e67690..98b177f67 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -1840,6 +1840,11 @@ BOOL update_read_cache_bitmap_order(wStream* s, CACHE_BITMAP_ORDER* cache_bitmap Stream_Read_UINT8(s, cache_bitmap->bitmapWidth); /* bitmapWidth (1 byte) */ Stream_Read_UINT8(s, cache_bitmap->bitmapHeight); /* bitmapHeight (1 byte) */ Stream_Read_UINT8(s, cache_bitmap->bitmapBpp); /* bitmapBpp (1 byte) */ + if ((cache_bitmap->bitmapBpp < 1) || (cache_bitmap->bitmapBpp > 32)) + { + fprintf(stderr, "%s: invalid bitmap bpp %d\n", __FUNCTION__, cache_bitmap->bitmapBpp); + return FALSE; + } Stream_Read_UINT16(s, cache_bitmap->bitmapLength); /* bitmapLength (2 bytes) */ Stream_Read_UINT16(s, cache_bitmap->cacheIndex); /* cacheIndex (2 bytes) */ @@ -2078,6 +2083,11 @@ BOOL update_read_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache_ bitmapData = &cache_bitmap_v3->bitmapData; Stream_Read_UINT8(s, bitmapData->bpp); + if ((bitmapData->bpp < 1) || (bitmapData->bpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, bitmapData->bpp); + return FALSE; + } Stream_Seek_UINT8(s); /* reserved1 (1 byte) */ Stream_Seek_UINT8(s); /* reserved2 (1 byte) */ Stream_Read_UINT8(s, bitmapData->codecID); /* codecID (1 byte) */ @@ -2682,6 +2692,11 @@ BOOL update_read_create_nine_grid_bitmap_order(wStream* s, CREATE_NINE_GRID_BITM return FALSE; Stream_Read_UINT8(s, create_nine_grid_bitmap->bitmapBpp); /* bitmapBpp (1 byte) */ + if ((create_nine_grid_bitmap->bitmapBpp < 1) || (create_nine_grid_bitmap->bitmapBpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, create_nine_grid_bitmap->bitmapBpp); + return FALSE; + } Stream_Read_UINT16(s, create_nine_grid_bitmap->bitmapId); /* bitmapId (2 bytes) */ nineGridInfo = &(create_nine_grid_bitmap->nineGridInfo); @@ -2717,6 +2732,12 @@ BOOL update_read_stream_bitmap_first_order(wStream* s, STREAM_BITMAP_FIRST_ORDER Stream_Read_UINT8(s, stream_bitmap_first->bitmapFlags); /* bitmapFlags (1 byte) */ Stream_Read_UINT8(s, stream_bitmap_first->bitmapBpp); /* bitmapBpp (1 byte) */ + if ((stream_bitmap_first->bitmapBpp < 1) || (stream_bitmap_first->bitmapBpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, stream_bitmap_first->bitmapBpp); + return FALSE; + } + Stream_Read_UINT16(s, stream_bitmap_first->bitmapType); /* bitmapType (2 bytes) */ Stream_Read_UINT16(s, stream_bitmap_first->bitmapWidth); /* bitmapWidth (2 bytes) */ Stream_Read_UINT16(s, stream_bitmap_first->bitmapHeight); /* bitmapHeigth (2 bytes) */ diff --git a/libfreerdp/core/surface.c b/libfreerdp/core/surface.c index 93f659607..af54e181a 100644 --- a/libfreerdp/core/surface.c +++ b/libfreerdp/core/surface.c @@ -38,6 +38,12 @@ static int update_recv_surfcmd_surface_bits(rdpUpdate* update, wStream* s, UINT3 Stream_Read_UINT16(s, cmd->destRight); Stream_Read_UINT16(s, cmd->destBottom); Stream_Read_UINT8(s, cmd->bpp); + if ((cmd->bpp < 1) || (cmd->bpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, cmd->bpp); + return FALSE; + } + Stream_Seek(s, 2); /* reserved1, reserved2 */ Stream_Read_UINT8(s, cmd->codecID); Stream_Read_UINT16(s, cmd->width); diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c index e9b7c33ef..2eefe2361 100644 --- a/libfreerdp/core/window.c +++ b/libfreerdp/core/window.c @@ -30,12 +30,19 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo) { + BYTE *newBitMask; if (Stream_GetRemainingLength(s) < 8) return FALSE; Stream_Read_UINT16(s, iconInfo->cacheEntry); /* cacheEntry (2 bytes) */ Stream_Read_UINT8(s, iconInfo->cacheId); /* cacheId (1 byte) */ Stream_Read_UINT8(s, iconInfo->bpp); /* bpp (1 byte) */ + if ((iconInfo->bpp < 1) || (iconInfo->bpp > 32)) + { + fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, iconInfo->bpp); + return FALSE; + } + Stream_Read_UINT16(s, iconInfo->width); /* width (2 bytes) */ Stream_Read_UINT16(s, iconInfo->height); /* height (2 bytes) */ @@ -62,10 +69,10 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo) return FALSE; /* bitsMask */ - if (iconInfo->bitsMask == NULL) - iconInfo->bitsMask = (BYTE*) malloc(iconInfo->cbBitsMask); - else - iconInfo->bitsMask = (BYTE*) realloc(iconInfo->bitsMask, iconInfo->cbBitsMask); + newBitMask = (BYTE*) realloc(iconInfo->bitsMask, iconInfo->cbBitsMask); + if (!newBitMask) + return FALSE; + iconInfo->bitsMask = newBitMask; Stream_Read(s, iconInfo->bitsMask, iconInfo->cbBitsMask); @@ -89,10 +96,10 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo) Stream_Read(s, iconInfo->colorTable, iconInfo->cbColorTable); /* bitsColor */ - if (iconInfo->bitsColor == NULL) - iconInfo->bitsColor = (BYTE*) malloc(iconInfo->cbBitsColor); - else - iconInfo->bitsColor = (BYTE*) realloc(iconInfo->bitsColor, iconInfo->cbBitsColor); + newBitMask = (BYTE *)realloc(iconInfo->bitsColor, iconInfo->cbBitsColor); + if (!newBitMask) + return FALSE; + iconInfo->bitsColor = newBitMask; Stream_Read(s, iconInfo->bitsColor, iconInfo->cbBitsColor);