diff --git a/channels/rdpsnd/server/rdpsnd_main.c b/channels/rdpsnd/server/rdpsnd_main.c index fe1598514..ad887ed35 100644 --- a/channels/rdpsnd/server/rdpsnd_main.c +++ b/channels/rdpsnd/server/rdpsnd_main.c @@ -199,13 +199,13 @@ static void* rdpsnd_server_thread(void* arg) buffer = NULL; BytesReturned = 0; - ChannelEvent = context->priv->ChannelHandle; + ChannelEvent = NULL; s = Stream_New(NULL, 4096); if (!s) return NULL; - if (WTSVirtualChannelQuery(ChannelEvent, WTSVirtualEventHandle, &buffer, &BytesReturned)) + if (WTSVirtualChannelQuery(context->priv->ChannelHandle, WTSVirtualEventHandle, &buffer, &BytesReturned)) { if (BytesReturned == sizeof(HANDLE)) CopyMemory(&ChannelEvent, buffer, sizeof(HANDLE)); @@ -214,7 +214,8 @@ static void* rdpsnd_server_thread(void* arg) } nCount = 0; - events[nCount++] = ChannelEvent; + if (ChannelEvent) + events[nCount++] = ChannelEvent; events[nCount++] = context->priv->StopEvent; if (!rdpsnd_server_send_formats(context, s)) @@ -230,7 +231,7 @@ static void* rdpsnd_server_thread(void* arg) Stream_SetPosition(s, 0); - if (!WTSVirtualChannelRead(ChannelEvent, 0, (PCHAR)Stream_Buffer(s), + if (!WTSVirtualChannelRead(context->priv->ChannelHandle, 0, (PCHAR)Stream_Buffer(s), Stream_Capacity(s), &BytesReturned)) { if (!BytesReturned) @@ -238,7 +239,7 @@ static void* rdpsnd_server_thread(void* arg) Stream_EnsureRemainingCapacity(s, BytesReturned); - if (!WTSVirtualChannelRead(ChannelEvent, 0, (PCHAR)Stream_Buffer(s), + if (!WTSVirtualChannelRead(context->priv->ChannelHandle, 0, (PCHAR)Stream_Buffer(s), Stream_Capacity(s), &BytesReturned)) break; } 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/fastpath.c b/libfreerdp/core/fastpath.c index 9efe2897e..c88e5c762 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -299,7 +299,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 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/transport.c b/libfreerdp/core/transport.c index 3390c5a8f..66dea82a0 100644 --- a/libfreerdp/core/transport.c +++ b/libfreerdp/core/transport.c @@ -726,6 +726,12 @@ int transport_read_layer(rdpTransport* transport, BYTE* data, int bytes) int read = 0; int status = -1; + if (!transport->frontBio) + { + transport->layer = TRANSPORT_LAYER_CLOSED; + return -1; + } + while (read < bytes) { status = BIO_read(transport->frontBio, data + read, bytes - read); @@ -738,7 +744,7 @@ int transport_read_layer(rdpTransport* transport, BYTE* data, int bytes) if (status < 0) { - if (!BIO_should_retry(transport->frontBio)) + if (!transport->frontBio || !BIO_should_retry(transport->frontBio)) { /* something unexpected happened, let's close */ transport->layer = TRANSPORT_LAYER_CLOSED; diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c index 15c5b9cf5..6a243aad0 100644 --- a/libfreerdp/core/update.c +++ b/libfreerdp/core/update.c @@ -286,16 +286,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) */ @@ -312,26 +328,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); } @@ -348,7 +403,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) @@ -387,7 +447,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 8c4dd8704..c67d04fc3 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 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); diff --git a/libfreerdp/utils/tcp.c b/libfreerdp/utils/tcp.c index 0938ec7b3..45293dbf3 100644 --- a/libfreerdp/utils/tcp.c +++ b/libfreerdp/utils/tcp.c @@ -187,6 +187,7 @@ int freerdp_tcp_write(int sockfd, BYTE* data, int length) int freerdp_tcp_wait_read(int sockfd) { fd_set fds; + struct timeval timeout; if (sockfd < 1) { @@ -196,7 +197,11 @@ int freerdp_tcp_wait_read(int sockfd) FD_ZERO(&fds); FD_SET(sockfd, &fds); - select(sockfd+1, &fds, NULL, NULL, NULL); + timeout.tv_sec = 5; + timeout.tv_usec = 0; + select(sockfd+1, &fds, NULL, NULL, &timeout); + if (!FD_ISSET(sockfd, &fds)) + return -1; return 0; } @@ -204,6 +209,7 @@ int freerdp_tcp_wait_read(int sockfd) int freerdp_tcp_wait_write(int sockfd) { fd_set fds; + struct timeval timeout; if (sockfd < 1) { @@ -213,7 +219,11 @@ int freerdp_tcp_wait_write(int sockfd) FD_ZERO(&fds); FD_SET(sockfd, &fds); - select(sockfd+1, NULL, &fds, NULL, NULL); + timeout.tv_sec = 5; + timeout.tv_usec = 0; + select(sockfd+1, NULL, &fds, NULL, &timeout); + if (!FD_ISSET(sockfd, &fds)) + return -1; return 0; }