CVE-2020-11096: Out-of-bounds Read (CWE-125)
value: In FreeRDP before version 2.1.2, there is a global OOB read in update_read_cache_bitmap_v3_order. As a workaround, one can disable bitmap cache with -bitmap-cache (default). This is fixed in version 2.1.2.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 6.5
Published: 2020-06-22T22:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/core/orders.c
Commit date: 2020-06-22 11:51:38 +0200
Commit message: Fixed OOB read in update_read_cache_bitmap_v3_order

CVE-2020-11096 thanks @antonio-morales for finding this.

Diff: diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c
index 90d79f54d6e..744bf3cd6ac 100644
--- a/libfreerdp/core/orders.c
+++ b/libfreerdp/core/orders.c
@@ -111,24 +111,78 @@ BYTE get_primary_drawing_order_field_bytes(UINT32 orderType, BOOL* pValid)
 			WLog_WARN(TAG, "Invalid orderType 0x%08X received", orderType);
 			return 0;
 	}
 }
 
-static const BYTE CBR2_BPP[] = { 0, 0, 0, 8, 16, 24, 32 };
-
-static const BYTE BPP_CBR2[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
-	                             0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };
-
-static const BYTE CBR23_BPP[] = { 0, 0, 0, 8, 16, 24, 32 };
-
-static const BYTE BPP_CBR23[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
-	                              0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };
-
-static const BYTE BMF_BPP[] = { 0, 1, 0, 8, 16, 24, 32, 0 };
+static BYTE get_cbr2_bpp(UINT32 bpp, BOOL* pValid)
+{
+	if (pValid)
+		*pValid = TRUE;
+	switch (bpp)
+	{
+		case 3:
+			return 8;
+		case 4:
+			return 16;
+		case 5:
+			return 24;
+		case 6:
+			return 32;
+		default:
+			WLog_WARN(TAG, "Invalid bpp %" PRIu32, bpp);
+			if (pValid)
+				*pValid = FALSE;
+			return 0;
+	}
+}
 
-static const BYTE BPP_BMF[] = { 0, 1, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
-	                            0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };
+static BYTE get_bmf_bpp(UINT32 bmf, BOOL* pValid)
+{
+	if (pValid)
+		*pValid = TRUE;
+	switch (bmf)
+	{
+		case 1:
+			return 1;
+		case 3:
+			return 8;
+		case 4:
+			return 16;
+		case 5:
+			return 24;
+		case 6:
+			return 32;
+		default:
+			WLog_WARN(TAG, "Invalid bmf %" PRIu32, bmf);
+			if (pValid)
+				*pValid = FALSE;
+			return 0;
+	}
+}
+static BYTE get_bpp_bmf(UINT32 bpp, BOOL* pValid)
+{
+	if (pValid)
+		*pValid = TRUE;
+	switch (bpp)
+	{
+		case 1:
+			return 1;
+		case 8:
+			return 3;
+		case 16:
+			return 4;
+		case 24:
+			return 5;
+		case 32:
+			return 6;
+		default:
+			WLog_WARN(TAG, "Invalid color depth %" PRIu32, bpp);
+			if (pValid)
+				*pValid = FALSE;
+			return 0;
+	}
+}
 
 static BOOL check_order_activated(wLog* log, rdpSettings* settings, const char* orderName,
                                   BOOL condition)
 {
 	if (!condition)
@@ -812,13 +866,15 @@ static INLINE BOOL update_read_brush(wStream* s, rdpBrush* brush, BYTE fieldFlag
 		Stream_Read_UINT8(s, brush->hatch);
 	}
 
 	if (brush->style & CACHED_BRUSH)
 	{
+		BOOL rc;
 		brush->index = brush->hatch;
-		brush->bpp = BMF_BPP[brush->style & 0x07];
-
+		brush->bpp = get_bmf_bpp(brush->style, &rc);
+		if (!rc)
+			return FALSE;
 		if (brush->bpp == 0)
 			brush->bpp = 1;
 	}
 
 	if (fieldFlags & ORDER_FIELD_05)
@@ -856,13 +912,15 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla
 		Stream_Write_UINT8(s, brush->style);
 	}
 
 	if (brush->style & CACHED_BRUSH)
 	{
+		BOOL rc;
 		brush->hatch = brush->index;
-		brush->bpp = BMF_BPP[brush->style & 0x07];
-
+		brush->bpp = get_bmf_bpp(brush->style, &rc);
+		if (!rc)
+			return FALSE;
 		if (brush->bpp == 0)
 			brush->bpp = 1;
 	}
 
 	if (fieldFlags & ORDER_FIELD_04)
@@ -2014,10 +2072,11 @@ BOOL update_write_cache_bitmap_order(wStream* s, const CACHE_BITMAP_ORDER* cache
 	return TRUE;
 }
 static CACHE_BITMAP_V2_ORDER* update_read_cache_bitmap_v2_order(rdpUpdate* update, wStream* s,
                                                                 BOOL compressed, UINT16 flags)
 {
+	BOOL rc;
 	BYTE bitsPerPixelId;
 	CACHE_BITMAP_V2_ORDER* cache_bitmap_v2;
 
 	if (!update || !s)
 		return NULL;
@@ -2028,11 +2087,13 @@ static CACHE_BITMAP_V2_ORDER* update_read_cache_bitmap_v2_order(rdpUpdate* updat
 		goto fail;
 
 	cache_bitmap_v2->cacheId = flags & 0x0003;
 	cache_bitmap_v2->flags = (flags & 0xFF80) >> 7;
 	bitsPerPixelId = (flags & 0x0078) >> 3;
-	cache_bitmap_v2->bitmapBpp = CBR2_BPP[bitsPerPixelId];
+	cache_bitmap_v2->bitmapBpp = get_cbr2_bpp(bitsPerPixelId, &rc);
+	if (!rc)
+		goto fail;
 
 	if (cache_bitmap_v2->flags & CBR2_PERSISTENT_KEY_PRESENT)
 	{
 		if (Stream_GetRemainingLength(s) < 8)
 			goto fail;
@@ -2107,17 +2168,20 @@ int update_approximate_cache_bitmap_v2_order(CACHE_BITMAP_V2_ORDER* cache_bitmap
 	return 64 + cache_bitmap_v2->bitmapLength;
 }
 BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache_bitmap_v2,
                                         BOOL compressed, UINT16* flags)
 {
+	BOOL rc;
 	BYTE bitsPerPixelId;
 
 	if (!Stream_EnsureRemainingCapacity(
 	        s, update_approximate_cache_bitmap_v2_order(cache_bitmap_v2, compressed, flags)))
 		return FALSE;
 
-	bitsPerPixelId = BPP_CBR2[cache_bitmap_v2->bitmapBpp];
+	bitsPerPixelId = get_bpp_bmf(cache_bitmap_v2->bitmapBpp, &rc);
+	if (!rc)
+		return FALSE;
 	*flags = (cache_bitmap_v2->cacheId & 0x0003) | (bitsPerPixelId << 3) |
 	         ((cache_bitmap_v2->flags << 7) & 0xFF80);
 
 	if (cache_bitmap_v2->flags & CBR2_PERSISTENT_KEY_PRESENT)
 	{
@@ -2175,10 +2239,11 @@ BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache
 	return TRUE;
 }
 static CACHE_BITMAP_V3_ORDER* update_read_cache_bitmap_v3_order(rdpUpdate* update, wStream* s,
                                                                 UINT16 flags)
 {
+	BOOL rc;
 	BYTE bitsPerPixelId;
 	BITMAP_DATA_EX* bitmapData;
 	UINT32 new_len;
 	BYTE* new_data;
 	CACHE_BITMAP_V3_ORDER* cache_bitmap_v3;
@@ -2192,11 +2257,13 @@ static CACHE_BITMAP_V3_ORDER* update_read_cache_bitmap_v3_order(rdpUpdate* updat
 		goto fail;
 
 	cache_bitmap_v3->cacheId = flags & 0x00000003;
 	cache_bitmap_v3->flags = (flags & 0x0000FF80) >> 7;
 	bitsPerPixelId = (flags & 0x00000078) >> 3;
-	cache_bitmap_v3->bpp = CBR23_BPP[bitsPerPixelId];
+	cache_bitmap_v3->bpp = get_cbr2_bpp(bitsPerPixelId, &rc);
+	if (!rc)
+		goto fail;
 
 	if (Stream_GetRemainingLength(s) < 21)
 		goto fail;
 
 	Stream_Read_UINT16(s, cache_bitmap_v3->cacheIndex); /* cacheIndex (2 bytes) */
@@ -2240,19 +2307,22 @@ int update_approximate_cache_bitmap_v3_order(CACHE_BITMAP_V3_ORDER* cache_bitmap
 	return 64 + bitmapData->length;
 }
 BOOL update_write_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache_bitmap_v3,
                                         UINT16* flags)
 {
+	BOOL rc;
 	BYTE bitsPerPixelId;
 	BITMAP_DATA_EX* bitmapData;
 
 	if (!Stream_EnsureRemainingCapacity(
 	        s, update_approximate_cache_bitmap_v3_order(cache_bitmap_v3, flags)))
 		return FALSE;
 
 	bitmapData = &cache_bitmap_v3->bitmapData;
-	bitsPerPixelId = BPP_CBR23[cache_bitmap_v3->bpp];
+	bitsPerPixelId = get_bpp_bmf(cache_bitmap_v3->bpp, &rc);
+	if (!rc)
+		return FALSE;
 	*flags = (cache_bitmap_v3->cacheId & 0x00000003) |
 	         ((cache_bitmap_v3->flags << 7) & 0x0000FF80) | ((bitsPerPixelId << 3) & 0x00000078);
 	Stream_Write_UINT16(s, cache_bitmap_v3->cacheIndex); /* cacheIndex (2 bytes) */
 	Stream_Write_UINT32(s, cache_bitmap_v3->key1);       /* key1 (4 bytes) */
 	Stream_Write_UINT32(s, cache_bitmap_v3->key2);       /* key2 (4 bytes) */
@@ -2572,10 +2642,11 @@ static BOOL update_compress_brush(wStream* s, const BYTE* input, BYTE bpp)
 	return FALSE;
 }
 static CACHE_BRUSH_ORDER* update_read_cache_brush_order(rdpUpdate* update, wStream* s, UINT16 flags)
 {
 	int i;
+	BOOL rc;
 	BYTE iBitmapFormat;
 	BOOL compressed = FALSE;
 	CACHE_BRUSH_ORDER* cache_brush = calloc(1, sizeof(CACHE_BRUSH_ORDER));
 
 	if (!cache_brush)
@@ -2585,14 +2656,14 @@ static CACHE_BRUSH_ORDER* update_read_cache_brush_order(rdpUpdate* update, wStre
 		goto fail;
 
 	Stream_Read_UINT8(s, cache_brush->index); /* cacheEntry (1 byte) */
 	Stream_Read_UINT8(s, iBitmapFormat);      /* iBitmapFormat (1 byte) */
 
-	if (iBitmapFormat >= ARRAYSIZE(BMF_BPP))
+	cache_brush->bpp = get_bmf_bpp(iBitmapFormat, &rc);
+	if (!rc)
 		goto fail;
 
-	cache_brush->bpp = BMF_BPP[iBitmapFormat];
 	Stream_Read_UINT8(s, cache_brush->cx);     /* cx (1 byte) */
 	Stream_Read_UINT8(s, cache_brush->cy);     /* cy (1 byte) */
 	Stream_Read_UINT8(s, cache_brush->style);  /* style (1 byte) */
 	Stream_Read_UINT8(s, cache_brush->length); /* iBytes (1 byte) */
 
@@ -2659,17 +2730,20 @@ int update_approximate_cache_brush_order(const CACHE_BRUSH_ORDER* cache_brush, U
 }
 BOOL update_write_cache_brush_order(wStream* s, const CACHE_BRUSH_ORDER* cache_brush, UINT16* flags)
 {
 	int i;
 	BYTE iBitmapFormat;
+	BOOL rc;
 	BOOL compressed = FALSE;
 
 	if (!Stream_EnsureRemainingCapacity(s,
 	                                    update_approximate_cache_brush_order(cache_brush, flags)))
 		return FALSE;
 
-	iBitmapFormat = BPP_BMF[cache_brush->bpp];
+	iBitmapFormat = get_bpp_bmf(cache_brush->bpp, &rc);
+	if (!rc)
+		return FALSE;
 	Stream_Write_UINT8(s, cache_brush->index);  /* cacheEntry (1 byte) */
 	Stream_Write_UINT8(s, iBitmapFormat);       /* iBitmapFormat (1 byte) */
 	Stream_Write_UINT8(s, cache_brush->cx);     /* cx (1 byte) */
 	Stream_Write_UINT8(s, cache_brush->cy);     /* cy (1 byte) */
 	Stream_Write_UINT8(s, cache_brush->style);  /* style (1 byte) */

Vulnerable code: static const BYTE CBR2_BPP[] = { 0, 0, 0, 8, 16, 24, 32 };

static const BYTE BPP_CBR2[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
	                             0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };

static const BYTE CBR23_BPP[] = { 0, 0, 0, 8, 16, 24, 32 };

static const BYTE BPP_CBR23[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
	                              0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };

static const BYTE BMF_BPP[] = { 0, 1, 0, 8, 16, 24, 32, 0 };
static const BYTE BPP_BMF[] = { 0, 1, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0,
	                            0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 };
		brush->bpp = BMF_BPP[brush->style & 0x07];

		brush->bpp = BMF_BPP[brush->style & 0x07];

	cache_bitmap_v2->bitmapBpp = CBR2_BPP[bitsPerPixelId];
	bitsPerPixelId = BPP_CBR2[cache_bitmap_v2->bitmapBpp];
	cache_bitmap_v3->bpp = CBR23_BPP[bitsPerPixelId];
	bitsPerPixelId = BPP_CBR23[cache_bitmap_v3->bpp];
	if (iBitmapFormat >= ARRAYSIZE(BMF_BPP))
	cache_brush->bpp = BMF_BPP[iBitmapFormat];
	iBitmapFormat = BPP_BMF[cache_brush->bpp];
Fixed code: static BYTE get_cbr2_bpp(UINT32 bpp, BOOL* pValid)
{
	if (pValid)
		*pValid = TRUE;
	switch (bpp)
	{
		case 3:
			return 8;
		case 4:
			return 16;
		case 5:
			return 24;
		case 6:
			return 32;
		default:
			WLog_WARN(TAG, "Invalid bpp %" PRIu32, bpp);
			if (pValid)
				*pValid = FALSE;
			return 0;
	}
}
static BYTE get_bmf_bpp(UINT32 bmf, BOOL* pValid)
{
	if (pValid)
		*pValid = TRUE;
	switch (bmf)
	{
		case 1:
			return 1;
		case 3:
			return 8;
		case 4:
			return 16;
		case 5:
			return 24;
		case 6:
			return 32;
		default:
			WLog_WARN(TAG, "Invalid bmf %" PRIu32, bmf);
			if (pValid)
				*pValid = FALSE;
			return 0;
	}
}
static BYTE get_bpp_bmf(UINT32 bpp, BOOL* pValid)
{
	if (pValid)
		*pValid = TRUE;
	switch (bpp)
	{
		case 1:
			return 1;
		case 8:
			return 3;
		case 16:
			return 4;
		case 24:
			return 5;
		case 32:
			return 6;
		default:
			WLog_WARN(TAG, "Invalid color depth %" PRIu32, bpp);
			if (pValid)
				*pValid = FALSE;
			return 0;
	}
}
		BOOL rc;
		brush->bpp = get_bmf_bpp(brush->style, &rc);
		if (!rc)
			return FALSE;
		BOOL rc;
		brush->bpp = get_bmf_bpp(brush->style, &rc);
		if (!rc)
			return FALSE;
	BOOL rc;
	cache_bitmap_v2->bitmapBpp = get_cbr2_bpp(bitsPerPixelId, &rc);
	if (!rc)
		goto fail;
	BOOL rc;
	bitsPerPixelId = get_bpp_bmf(cache_bitmap_v2->bitmapBpp, &rc);
	if (!rc)
		return FALSE;
	BOOL rc;
	cache_bitmap_v3->bpp = get_cbr2_bpp(bitsPerPixelId, &rc);
	if (!rc)
		goto fail;
	BOOL rc;
	bitsPerPixelId = get_bpp_bmf(cache_bitmap_v3->bpp, &rc);
	if (!rc)
		return FALSE;
	BOOL rc;
	cache_brush->bpp = get_bmf_bpp(iBitmapFormat, &rc);
	if (!rc)
	BOOL rc;
	iBitmapFormat = get_bpp_bmf(cache_brush->bpp, &rc);
	if (!rc)
		return FALSE;