| From 5ad707ddf33d1d785a8ca1fbeec91d2eee985820 Mon Sep 17 00:00:00 2001 |
| From: Hardening <rdp.effort@gmail.com> |
| Date: Fri, 6 Jun 2014 23:24:16 +0200 |
| Subject: [PATCH] Fix CVE-2014-0250 |
| |
| This patch fixes CVE-2014-0250 by checking width, height and bpp when |
| receiving a new pointer. |
| --- |
| client/X11/xf_cliprdr.c | 11 +++++-- |
| libfreerdp/core/fastpath.c | 2 +- |
| libfreerdp/core/orders.c | 21 ++++++++++++ |
| libfreerdp/core/surface.c | 6 ++++ |
| libfreerdp/core/update.c | 82 +++++++++++++++++++++++++++++++++++++++------- |
| libfreerdp/core/update.h | 2 +- |
| libfreerdp/core/window.c | 5 +++ |
| 7 files changed, 114 insertions(+), 15 deletions(-) |
| |
| diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c |
| index 19c4332..8fb49f9 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/fastpath.c b/libfreerdp/core/fastpath.c |
| index 8448160..dcc7117 100644 |
| --- a/libfreerdp/core/fastpath.c |
| +++ b/libfreerdp/core/fastpath.c |
| @@ -254,7 +254,7 @@ static int fastpath_recv_update(rdpFastPath* fastpath, BYTE updateCode, UINT32 s |
| break; |
| |
| case FASTPATH_UPDATETYPE_COLOR: |
| - if (!update_read_pointer_color(s, &pointer->pointer_color)) |
| + if (!update_read_pointer_color(s, &pointer->pointer_color, 24)) |
| return -1; |
| IFCALL(pointer->PointerColor, context, &pointer->pointer_color); |
| break; |
| diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c |
| index e5cc520..f22407f 100644 |
| --- a/libfreerdp/core/orders.c |
| +++ b/libfreerdp/core/orders.c |
| @@ -1830,6 +1830,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) */ |
| |
| @@ -2068,6 +2073,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) */ |
| @@ -2672,6 +2682,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); |
| @@ -2707,6 +2722,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 7d0fb22..992a3dd 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/update.c b/libfreerdp/core/update.c |
| index c4eaede..27c36e3 100644 |
| --- a/libfreerdp/core/update.c |
| +++ b/libfreerdp/core/update.c |
| @@ -219,16 +219,32 @@ BOOL update_read_pointer_system(wStream* s, POINTER_SYSTEM_UPDATE* pointer_syste |
| return TRUE; |
| } |
| |
| -BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color) |
| +BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color, int xorBpp) |
| { |
| + BYTE *newMask; |
| + int scanlineSize; |
| + |
| if (Stream_GetRemainingLength(s) < 14) |
| return FALSE; |
| |
| Stream_Read_UINT16(s, pointer_color->cacheIndex); /* cacheIndex (2 bytes) */ |
| Stream_Read_UINT16(s, pointer_color->xPos); /* xPos (2 bytes) */ |
| Stream_Read_UINT16(s, pointer_color->yPos); /* yPos (2 bytes) */ |
| + |
| + /** |
| + * As stated in 2.2.9.1.1.4.4 Color Pointer Update: |
| + * The maximum allowed pointer width/height is 96 pixels if the client indicated support |
| + * for large pointers by setting the LARGE_POINTER_FLAG (0x00000001) in the Large |
| + * Pointer Capability Set (section 2.2.7.2.7). If the LARGE_POINTER_FLAG was not |
| + * set, the maximum allowed pointer width/height is 32 pixels. |
| + * |
| + * So we check for a maximum of 96 for CVE-2014-0250. |
| + */ |
| Stream_Read_UINT16(s, pointer_color->width); /* width (2 bytes) */ |
| Stream_Read_UINT16(s, pointer_color->height); /* height (2 bytes) */ |
| + if ((pointer_color->width > 96) || (pointer_color->height > 96)) |
| + return FALSE; |
| + |
| Stream_Read_UINT16(s, pointer_color->lengthAndMask); /* lengthAndMask (2 bytes) */ |
| Stream_Read_UINT16(s, pointer_color->lengthXorMask); /* lengthXorMask (2 bytes) */ |
| |
| @@ -245,26 +261,65 @@ BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color) |
| |
| if (pointer_color->lengthXorMask > 0) |
| { |
| + /** |
| + * Spec states that: |
| + * |
| + * xorMaskData (variable): A variable-length array of bytes. Contains the 24-bpp, bottom-up |
| + * XOR mask scan-line data. The XOR mask is padded to a 2-byte boundary for each encoded |
| + * scan-line. For example, if a 3x3 pixel cursor is being sent, then each scan-line will consume 10 |
| + * bytes (3 pixels per scan-line multiplied by 3 bytes per pixel, rounded up to the next even |
| + * number of bytes). |
| + * |
| + * In fact instead of 24-bpp, the bpp parameter is given by the containing packet. |
| + */ |
| if (Stream_GetRemainingLength(s) < pointer_color->lengthXorMask) |
| return FALSE; |
| |
| - if (!pointer_color->xorMaskData) |
| - pointer_color->xorMaskData = malloc(pointer_color->lengthXorMask); |
| - else |
| - pointer_color->xorMaskData = realloc(pointer_color->xorMaskData, pointer_color->lengthXorMask); |
| + scanlineSize = (7 + xorBpp * pointer_color->width) / 8; |
| + scanlineSize = ((scanlineSize + 1) / 2) * 2; |
| + if (scanlineSize * pointer_color->height != pointer_color->lengthXorMask) |
| + { |
| + fprintf(stderr, "%s: invalid lengthXorMask: width=%d height=%d, %d instead of %d\n", __FUNCTION__, |
| + pointer_color->width, pointer_color->height, |
| + pointer_color->lengthXorMask, scanlineSize * pointer_color->height); |
| + return FALSE; |
| + } |
| + |
| + newMask = realloc(pointer_color->xorMaskData, pointer_color->lengthXorMask); |
| + if (!newMask) |
| + return FALSE; |
| + |
| + pointer_color->xorMaskData = newMask; |
| |
| Stream_Read(s, pointer_color->xorMaskData, pointer_color->lengthXorMask); |
| } |
| |
| if (pointer_color->lengthAndMask > 0) |
| { |
| + /** |
| + * andMaskData (variable): A variable-length array of bytes. Contains the 1-bpp, bottom-up |
| + * AND mask scan-line data. The AND mask is padded to a 2-byte boundary for each encoded |
| + * scan-line. For example, if a 7x7 pixel cursor is being sent, then each scan-line will consume 2 |
| + * bytes (7 pixels per scan-line multiplied by 1 bpp, rounded up to the next even number of |
| + * bytes). |
| + */ |
| if (Stream_GetRemainingLength(s) < pointer_color->lengthAndMask) |
| return FALSE; |
| |
| - if (!pointer_color->andMaskData) |
| - pointer_color->andMaskData = malloc(pointer_color->lengthAndMask); |
| - else |
| - pointer_color->andMaskData = realloc(pointer_color->andMaskData, pointer_color->lengthAndMask); |
| + scanlineSize = ((7 + pointer_color->width) / 8); |
| + scanlineSize = ((1 + scanlineSize) / 2) * 2; |
| + if (scanlineSize * pointer_color->height != pointer_color->lengthAndMask) |
| + { |
| + fprintf(stderr, "%s: invalid lengthAndMask: %d instead of %d\n", __FUNCTION__, |
| + pointer_color->lengthAndMask, scanlineSize * pointer_color->height); |
| + return FALSE; |
| + } |
| + |
| + newMask = realloc(pointer_color->andMaskData, pointer_color->lengthAndMask); |
| + if (!newMask) |
| + return FALSE; |
| + |
| + pointer_color->andMaskData = newMask; |
| |
| Stream_Read(s, pointer_color->andMaskData, pointer_color->lengthAndMask); |
| } |
| @@ -281,7 +336,12 @@ BOOL update_read_pointer_new(wStream* s, POINTER_NEW_UPDATE* pointer_new) |
| return FALSE; |
| |
| Stream_Read_UINT16(s, pointer_new->xorBpp); /* xorBpp (2 bytes) */ |
| - return update_read_pointer_color(s, &pointer_new->colorPtrAttr); /* colorPtrAttr */ |
| + if ((pointer_new->xorBpp < 1) || (pointer_new->xorBpp > 32)) |
| + { |
| + fprintf(stderr, "%s: invalid xorBpp %d\n", __FUNCTION__, pointer_new->xorBpp); |
| + return FALSE; |
| + } |
| + return update_read_pointer_color(s, &pointer_new->colorPtrAttr, pointer_new->xorBpp); /* colorPtrAttr */ |
| } |
| |
| BOOL update_read_pointer_cached(wStream* s, POINTER_CACHED_UPDATE* pointer_cached) |
| @@ -320,7 +380,7 @@ BOOL update_recv_pointer(rdpUpdate* update, wStream* s) |
| break; |
| |
| case PTR_MSG_TYPE_COLOR: |
| - if (!update_read_pointer_color(s, &pointer->pointer_color)) |
| + if (!update_read_pointer_color(s, &pointer->pointer_color, 24)) |
| return FALSE; |
| IFCALL(pointer->PointerColor, context, &pointer->pointer_color); |
| break; |
| diff --git a/libfreerdp/core/update.h b/libfreerdp/core/update.h |
| index c3088f2..d6c2d59 100644 |
| --- a/libfreerdp/core/update.h |
| +++ b/libfreerdp/core/update.h |
| @@ -53,7 +53,7 @@ BOOL update_recv(rdpUpdate* update, wStream* s); |
| |
| BOOL update_read_pointer_position(wStream* s, POINTER_POSITION_UPDATE* pointer_position); |
| BOOL update_read_pointer_system(wStream* s, POINTER_SYSTEM_UPDATE* pointer_system); |
| -BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color); |
| +BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color, int xorBpp); |
| BOOL update_read_pointer_new(wStream* s, POINTER_NEW_UPDATE* pointer_new); |
| BOOL update_read_pointer_cached(wStream* s, POINTER_CACHED_UPDATE* pointer_cached); |
| |
| diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c |
| index 7422f5b..c10fa33 100644 |
| --- a/libfreerdp/core/window.c |
| +++ b/libfreerdp/core/window.c |
| @@ -35,6 +35,11 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* icon_info) |
| Stream_Read_UINT16(s, icon_info->cacheEntry); /* cacheEntry (2 bytes) */ |
| Stream_Read_UINT8(s, icon_info->cacheId); /* cacheId (1 byte) */ |
| Stream_Read_UINT8(s, icon_info->bpp); /* bpp (1 byte) */ |
| + if ((icon_info->bpp < 1) || (icon_info->bpp > 32)) |
| + { |
| + fprintf(stderr, "%s: invalid bpp %d\n", __FUNCTION__, icon_info->bpp); |
| + return FALSE; |
| + } |
| Stream_Read_UINT16(s, icon_info->width); /* width (2 bytes) */ |
| Stream_Read_UINT16(s, icon_info->height); /* height (2 bytes) */ |
| |
| -- |
| 1.9.3 |
| |