File freerdp-CVE-2023-39350-to-2023-40589.patch of Package freerdp

Overview Repositories Revisions Requests Users Attributes Meta

File freerdp-CVE-2023-39350-to-2023-40589.patch of Package freerdp (Revision 3979d1be6a7ae3c50690615777528b21)

Currently displaying revision 3979d1be6a7ae3c50690615777528b21 , Show latest

From 660f905eebea58e86965957c20ef50d3b659762a Mon Sep 17 00:00:00 2001
From: Armin Novak <anovak@thincast.com>
Date: Sat, 5 Aug 2023 08:57:28 +0200
Subject: [PATCH 01/15] [coded,rfx] check indices are within range

reported by @pwn2carr
---
 libfreerdp/codec/rfx.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c
index b7db992ee..6ab8dd8b4 100644
--- a/libfreerdp/codec/rfx.c
+++ b/libfreerdp/codec/rfx.c
@@ -934,6 +934,32 @@ static BOOL rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa
 		Stream_Read_UINT8(&sub, tile->quantIdxY);  /* quantIdxY (1 byte) */
 		Stream_Read_UINT8(&sub, tile->quantIdxCb); /* quantIdxCb (1 byte) */
 		Stream_Read_UINT8(&sub, tile->quantIdxCr); /* quantIdxCr (1 byte) */
+
+		if (tile->quantIdxY >= context->numQuant)
+		{
+			WLog_Print(context->priv->log, WLOG_ERROR,
+				   "quantIdxY %" PRIu8 " >= numQuant %" PRIu8, tile->quantIdxY,
+				   context->numQuant);
+			rc = FALSE;
+			break;
+		}
+		if (tile->quantIdxCb >= context->numQuant)
+		{
+			WLog_Print(context->priv->log, WLOG_ERROR,
+				   "quantIdxCb %" PRIu8 " >= numQuant %" PRIu8, tile->quantIdxCb,
+				   context->numQuant);
+			rc = FALSE;
+			break;
+		}
+		if (tile->quantIdxCr >= context->numQuant)
+		{
+			WLog_Print(context->priv->log, WLOG_ERROR,
+				   "quantIdxCr %" PRIu8 " >= numQuant %" PRIu8, tile->quantIdxCr,
+				   context->numQuant);
+			rc = FALSE;
+			break;
+		}
+
 		Stream_Read_UINT16(&sub, tile->xIdx);      /* xIdx (2 bytes) */
 		Stream_Read_UINT16(&sub, tile->yIdx);      /* yIdx (2 bytes) */
 		Stream_Read_UINT16(&sub, tile->YLen);      /* YLen (2 bytes) */
-- 
2.42.1

From 836e6821277ec6214673a6f88acd0dc1b13c6aee Mon Sep 17 00:00:00 2001
From: Armin Novak <anovak@thincast.com>
Date: Sat, 5 Aug 2023 09:29:19 +0200
Subject: [PATCH 02/15] [codec,rfx] free content of currentMessage on fail

---
 libfreerdp/codec/rfx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c
index 6ab8dd8b4..3cd34fc77 100644
--- a/libfreerdp/codec/rfx.c
+++ b/libfreerdp/codec/rfx.c
@@ -1249,6 +1249,11 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length,
 		region16_uninit(&clippingRects);
 		return TRUE;
 	}
+	else
+	{
+		rfx_message_free(context, message);
+		context->currentMessage.freeArray = TRUE;
+	}
 
 	return FALSE;
 }
-- 
2.42.1

From ceb71ad18a0796444ced47745174752814a627b7 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Fri, 4 Aug 2023 13:55:40 +0200
Subject: [PATCH 03/15] [codec,rfx] fix possible out of bound read

Allows malicious servers to crash FreeRDP based clients
reported by @pwn2carr
---
 libfreerdp/codec/rfx.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c
index 3cd34fc77..5afff85e2 100644
--- a/libfreerdp/codec/rfx.c
+++ b/libfreerdp/codec/rfx.c
@@ -1043,10 +1043,10 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length,
                          UINT32 top, BYTE* dst, UINT32 dstFormat, UINT32 dstStride,
                          UINT32 dstHeight, REGION16* invalidRegion)
 {
-	REGION16 updateRegion;
+	REGION16 updateRegion = { 0 };
 	UINT32 blockLen;
 	UINT32 blockType;
-	wStream inStream, *s = &inStream;
+	wStream inStream = { 0 }, *s = &inStream;
 	BOOL ok = TRUE;
 	RFX_MESSAGE* message;
 
@@ -1061,8 +1061,10 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length,
 
 	while (ok && Stream_GetRemainingLength(s) > 6)
 	{
-		wStream subStream;
+		wStream subStream = { 0 };
 		size_t extraBlockLen = 0;
+		UINT32 blockLen = 0;
+		UINT32 blockType = 0;
 
 		/* RFX_BLOCKT */
 		Stream_Read_UINT16(s, blockType); /* blockType (2 bytes) */
@@ -1091,8 +1093,8 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length,
 		if (blockType >= WBT_CONTEXT && blockType <= WBT_EXTENSION)
 		{
 			/* RFX_CODEC_CHANNELT */
-			UINT8 codecId;
-			UINT8 channelId;
+			UINT8 codecId = 0;
+			UINT8 channelId = 0;
 
 			if (Stream_GetRemainingLength(s) < 2)
 				return FALSE;
@@ -1130,8 +1132,18 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length,
 			}
 		}
 
-		Stream_StaticInit(&subStream, Stream_Pointer(s), blockLen - (6 + extraBlockLen));
-		Stream_Seek(s, blockLen - (6 + extraBlockLen));
+		const size_t blockLenNoHeader = blockLen - 6;
+		if (blockLenNoHeader < extraBlockLen)
+		{
+			WLog_Print(context->priv->log, WLOG_ERROR,
+			           "blockLen too small(%" PRIu32 "), must be >= 6 + %" PRIu16, blockLen,
+			           extraBlockLen);
+			return FALSE;
+		}
+
+		const size_t subStreamLen = blockLenNoHeader - extraBlockLen;
+		Stream_StaticInit(&subStream, Stream_Pointer(s), subStreamLen);
+		Stream_Seek(s, subStreamLen);
 
 		switch (blockType)
 		{
-- 
2.42.1

From 081f95fb6f6088f0d8c83487c1b6de08186ee04e Mon Sep 17 00:00:00 2001
From: Armin Novak <anovak@thincast.com>
Date: Fri, 4 Aug 2023 16:42:05 +0200
Subject: [PATCH 04/15] [codec,color] fix freerdp_image_fill

in case width or height == 0 out of bounds write might happen.
reported by @pwn2carr
---
 libfreerdp/codec/color.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c
index 988cd1569..61d3c6428 100644
--- a/libfreerdp/codec/color.c
+++ b/libfreerdp/codec/color.c
@@ -723,9 +723,17 @@ BOOL freerdp_image_fill(BYTE* pDstData, DWORD DstFormat, UINT32 nDstStep, UINT32
                         UINT32 nYDst, UINT32 nWidth, UINT32 nHeight, UINT32 color)
 {
 	UINT32 x, y;
+	if ((nWidth == 0) || (nHeight == 0))
+		return TRUE;
 	const UINT32 bpp = GetBytesPerPixel(DstFormat);
-	BYTE* pFirstDstLine = &pDstData[nYDst * nDstStep];
-	BYTE* pFirstDstLineXOffset = &pFirstDstLine[nXDst * bpp];
+	BYTE* pFirstDstLine;
+	BYTE* pFirstDstLineXOffset;
+
+	if (nDstStep == 0)
+		nDstStep = (nXDst + nWidth) * GetBytesPerPixel(DstFormat);
+
+	pFirstDstLine = &pDstData[nYDst * nDstStep];
+	pFirstDstLineXOffset = &pFirstDstLine[nXDst * bpp];
 
 	for (x = 0; x < nWidth; x++)
 	{
-- 
2.42.1

From f82372c43fe078989c8b96619831a8272781d3f5 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Tue, 22 Aug 2023 10:48:57 +0200
Subject: [PATCH 05/15] [codec,nsc] fix input length validation

---
 libfreerdp/codec/nsc.c       | 33 +++++++++++++++++++++++++++++----
 libfreerdp/codec/nsc_types.h |  1 +
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c
index 6627e0b63..c5e8c8597 100644
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -111,12 +111,17 @@ static BOOL nsc_decode(NSC_CONTEXT* context)
 	return TRUE;
 }
 
-static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalSize)
+static BOOL nsc_rle_decode(const BYTE* in, size_t inSize, BYTE* out, UINT32 outSize,
+                           UINT32 originalSize)
 {
 	UINT32 left = originalSize;
 
 	while (left > 4)
 	{
+		if (inSize < 1)
+			return FALSE;
+		inSize--;
+
 		const BYTE value = *in++;
 		UINT32 len = 0;
 
@@ -129,17 +134,26 @@ static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalS
 			*out++ = value;
 			left--;
 		}
+		else if (inSize < 1)
+			return FALSE;
 		else if (value == *in)
 		{
+			inSize--;
 			in++;
 
-			if (*in < 0xFF)
+			if (inSize < 1)
+				return FALSE;
+			else if (*in < 0xFF)
 			{
+				inSize--;
 				len = (UINT32)*in++;
 				len += 2;
 			}
 			else
 			{
+				if (inSize < 5)
+					return FALSE;
+				inSize -= 5;
 				in++;
 				len = ((UINT32)(*in++));
 				len |= ((UINT32)(*in++)) << 8U;
@@ -169,6 +183,8 @@ static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalS
 	if ((outSize < 4) || (left < 4))
 		return FALSE;
 
+	if (inSize < 4)
+		return FALSE;
 	memcpy(out, in, 4);
 	return TRUE;
 }
@@ -184,12 +200,17 @@ static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 		return FALSE;
 
 	rle = context->Planes;
+	size_t rleSize = context->PlanesSize;
+	WINPR_ASSERT(rle);
 
 	for (i = 0; i < 4; i++)
 	{
 		originalSize = context->OrgByteCount[i];
 		planeSize = context->PlaneByteCount[i];
 
+		if (rleSize < planeSize)
+			return FALSE;
+
 		if (planeSize == 0)
 		{
 			if (context->priv->PlaneBuffersLength < originalSize)
@@ -199,7 +220,7 @@ static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 		}
 		else if (planeSize < originalSize)
 		{
-			if (!nsc_rle_decode(rle, context->priv->PlaneBuffers[i],
+			if (!nsc_rle_decode(rle, rleSize, context->priv->PlaneBuffers[i],
 			                    context->priv->PlaneBuffersLength, originalSize))
 				return FALSE;
 		}
@@ -208,6 +229,9 @@ static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 			if (context->priv->PlaneBuffersLength < originalSize)
 				return FALSE;
 
+			if (rleSize < originalSize)
+				return FALSE;
+
 			CopyMemory(context->priv->PlaneBuffers[i], rle, originalSize);
 		}
 
@@ -231,7 +255,8 @@ static BOOL nsc_stream_initialize(NSC_CONTEXT* context, wStream* s)
 	Stream_Read_UINT8(s, context->ChromaSubsamplingLevel); /* ChromaSubsamplingLevel (1 byte) */
 	Stream_Seek(s, 2);                                     /* Reserved (2 bytes) */
 	context->Planes = Stream_Pointer(s);
-	return TRUE;
+	context->PlanesSize = total;
+	return Stream_CheckAndLogRequiredLengthWLog(context->priv->log, s, total);
 }
 
 static BOOL nsc_context_initialize(NSC_CONTEXT* context, wStream* s)
diff --git a/libfreerdp/codec/nsc_types.h b/libfreerdp/codec/nsc_types.h
index 14c1a946b..5a6506bae 100644
--- a/libfreerdp/codec/nsc_types.h
+++ b/libfreerdp/codec/nsc_types.h
@@ -61,6 +61,7 @@ struct _NSC_CONTEXT
 	UINT32 BitmapDataLength;
 
 	BYTE* Planes;
+	size_t PlanesSize;
 	UINT32 PlaneByteCount[4];
 	UINT32 ColorLossLevel;
 	UINT32 ChromaSubsamplingLevel;
-- 
2.42.1

From 6cd7f077c2127dd8fc7974ce970b303ad9156c38 Mon Sep 17 00:00:00 2001
From: Armin Novak <anovak@thincast.com>
Date: Sat, 5 Aug 2023 11:31:08 +0200
Subject: [PATCH 06/15] [core,orders] fix checks for multi opaque rect

---
 libfreerdp/core/orders.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c
index 3595818f7..726f7df70 100644
--- a/libfreerdp/core/orders.c
+++ b/libfreerdp/core/orders.c
@@ -27,6 +27,7 @@
 
 #include <winpr/wtypes.h>
 #include <winpr/crt.h>
+#include <winpr/assert.h>
 
 #include <freerdp/api.h>
 #include <freerdp/log.h>
@@ -1353,7 +1354,7 @@ static BOOL update_read_multi_dstblt_order(wStream* s, const ORDER_INFO* orderIn
 		return update_read_delta_rects(s, multi_dstblt->rectangles, &multi_dstblt->numRectangles);
 	}
 
-	return TRUE;
+	return multi_dstblt->numRectangles == 0;
 }
 static BOOL update_read_multi_patblt_order(wStream* s, const ORDER_INFO* orderInfo,
                                            MULTI_PATBLT_ORDER* multi_patblt)
@@ -1381,12 +1382,19 @@ static BOOL update_read_multi_patblt_order(wStream* s, const ORDER_INFO* orderIn
 		if (!update_read_delta_rects(s, multi_patblt->rectangles, &multi_patblt->numRectangles))
 			return FALSE;
 	}
+	else if (multi_patblt->numRectangles != 0)
+		return FALSE;
 
 	return TRUE;
 }
 static BOOL update_read_multi_scrblt_order(wStream* s, const ORDER_INFO* orderInfo,
                                            MULTI_SCRBLT_ORDER* multi_scrblt)
 {
+	WINPR_ASSERT(orderInfo);
+	WINPR_ASSERT(multi_scrblt);
+
+	multi_scrblt->numRectangles = 0;
+
 	ORDER_FIELD_COORD(1, multi_scrblt->nLeftRect);
 	ORDER_FIELD_COORD(2, multi_scrblt->nTopRect);
 	ORDER_FIELD_COORD(3, multi_scrblt->nWidth);
@@ -1405,7 +1413,7 @@ static BOOL update_read_multi_scrblt_order(wStream* s, const ORDER_INFO* orderIn
 		return update_read_delta_rects(s, multi_scrblt->rectangles, &multi_scrblt->numRectangles);
 	}
 
-	return TRUE;
+	return multi_scrblt->numRectangles == 0;
 }
 static BOOL update_read_multi_opaque_rect_order(wStream* s, const ORDER_INFO* orderInfo,
                                                 MULTI_OPAQUE_RECT_ORDER* multi_opaque_rect)
@@ -1455,7 +1463,7 @@ static BOOL update_read_multi_opaque_rect_order(wStream* s, const ORDER_INFO* or
 		                               &multi_opaque_rect->numRectangles);
 	}
 
-	return TRUE;
+	return multi_opaque_rect->numRectangles == 0;
 }
 static BOOL update_read_multi_draw_nine_grid_order(wStream* s, const ORDER_INFO* orderInfo,
                                                    MULTI_DRAW_NINE_GRID_ORDER* multi_draw_nine_grid)
@@ -1477,7 +1485,7 @@ static BOOL update_read_multi_draw_nine_grid_order(wStream* s, const ORDER_INFO*
 		                               &multi_draw_nine_grid->nDeltaEntries);
 	}
 
-	return TRUE;
+	return multi_draw_nine_grid->nDeltaEntries == 0;
 }
 static BOOL update_read_line_to_order(wStream* s, const ORDER_INFO* orderInfo,
                                       LINE_TO_ORDER* line_to)
-- 
2.42.1

From 748e40cbb76feb45a62dad82e222f6f57ec2a3d4 Mon Sep 17 00:00:00 2001
From: Armin Novak <armin.novak@thincast.com>
Date: Mon, 21 Aug 2023 14:30:11 +0200
Subject: [PATCH 07/15] [codec,zgfx] fix cBitsRemaining calculation

fixed out of bound read reported by @pwn2carr
---
 libfreerdp/codec/zgfx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libfreerdp/codec/zgfx.c b/libfreerdp/codec/zgfx.c
index 04ddeadb2..4489b3798 100644
--- a/libfreerdp/codec/zgfx.c
+++ b/libfreerdp/codec/zgfx.c
@@ -259,7 +259,11 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 	zgfx->pbInputCurrent = pbSegment;
 	zgfx->pbInputEnd = &pbSegment[cbSegment - 1];
 	/* NumberOfBitsToDecode = ((NumberOfBytesToDecode - 1) * 8) - ValueOfLastByte */
-	zgfx->cBitsRemaining = 8 * (cbSegment - 1) - *zgfx->pbInputEnd;
+	const UINT32 bits = 8u * (cbSegment - 1u);
+	if (bits < *zgfx->pbInputEnd)
+		return FALSE;
+
+	zgfx->cBitsRemaining = bits - *zgfx->pbInputEnd;
 	zgfx->cBitsCurrent = 0;
 	zgfx->BitsCurrent = 0;
 
-- 
2.42.1

From 39eb23723ceee10c0033287683c90b1e1bc88f94 Mon Sep 17 00:00:00 2001
From: Youkou Tenhouin <youkou@tenhou.in>
Date: Tue, 14 Nov 2023 14:00:46 +0800
Subject: [PATCH 08/15] SLE: fix CVE-2023-40186

---
 libfreerdp/gdi/gfx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c
index f368165b6..0afad2d8b 100644
--- a/libfreerdp/gdi/gfx.c
+++ b/libfreerdp/gdi/gfx.c
@@ -991,7 +991,7 @@ static UINT gdi_CreateSurface(RdpgfxClientContext* context,
 	}
 
 	surface->scanline = gfx_align_scanline(surface->width * 4, 16);
-	surface->data = (BYTE*)_aligned_malloc(surface->scanline * surface->height, 16);
+	surface->data = (BYTE*)_aligned_malloc(1ull * surface->scanline * surface->height, 16);
 
 	if (!surface->data)
 	{
-- 
2.42.1

From a4d60e5774c558f823e3b1979004b67cec7b2e26 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Tue, 22 Aug 2023 10:48:57 +0200
Subject: [PATCH 09/15] [codec,nsc] fix input length validation

---
 libfreerdp/codec/nsc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c
index c5e8c8597..cdd2c0eae 100644
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -29,6 +29,7 @@
 #include <string.h>
 
 #include <winpr/crt.h>
+#include <winpr/assert.h>
 
 #include <freerdp/codec/nsc.h>
 #include <freerdp/codec/color.h>
@@ -208,6 +209,9 @@ static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 		originalSize = context->OrgByteCount[i];
 		planeSize = context->PlaneByteCount[i];
 
+		if (rleSize < planeSize)
+			return FALSE;
+
 		if (rleSize < planeSize)
 			return FALSE;
 
@@ -248,15 +252,18 @@ static BOOL nsc_stream_initialize(NSC_CONTEXT* context, wStream* s)
 	if (Stream_GetRemainingLength(s) < 20)
 		return FALSE;
 
-	for (i = 0; i < 4; i++)
+	size_t total = 0;
+	for (i = 0; i < 4; i++) {
 		Stream_Read_UINT32(s, context->PlaneByteCount[i]);
+		total += context->PlaneByteCount[i];
+	}
 
 	Stream_Read_UINT8(s, context->ColorLossLevel);         /* ColorLossLevel (1 byte) */
 	Stream_Read_UINT8(s, context->ChromaSubsamplingLevel); /* ChromaSubsamplingLevel (1 byte) */
 	Stream_Seek(s, 2);                                     /* Reserved (2 bytes) */
 	context->Planes = Stream_Pointer(s);
 	context->PlanesSize = total;
-	return Stream_CheckAndLogRequiredLengthWLog(context->priv->log, s, total);
+	return Stream_GetRemainingLength(s) >= total;
 }
 
 static BOOL nsc_context_initialize(NSC_CONTEXT* context, wStream* s)
-- 
2.42.1

From 364850e63a15bbd8db87b079b60e8022f5c54a72 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Tue, 22 Aug 2023 14:44:57 +0200
Subject: [PATCH 10/15] [codec,clear] fix missing bounds checks

---
 libfreerdp/codec/clear.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/libfreerdp/codec/clear.c b/libfreerdp/codec/clear.c
index 083f1aa97..bc8f1821e 100644
--- a/libfreerdp/codec/clear.c
+++ b/libfreerdp/codec/clear.c
@@ -591,11 +591,10 @@ static BOOL resize_vbar_entry(CLEAR_CONTEXT* clear, CLEAR_VBAR_ENTRY* vBarEntry)
 static BOOL clear_decompress_bands_data(CLEAR_CONTEXT* clear, wStream* s, UINT32 bandsByteCount,
                                         UINT32 nWidth, UINT32 nHeight, BYTE* pDstData,
                                         UINT32 DstFormat, UINT32 nDstStep, UINT32 nXDst,
-                                        UINT32 nYDst)
+                                        UINT32 nYDst, UINT32 nDstWidth, UINT32 nDstHeight)
 {
 	UINT32 i, y;
-	UINT32 count;
-	UINT32 suboffset;
+	UINT32 suboffset = 0;
 	UINT32 nXDstRel;
 	UINT32 nYDstRel;
 
@@ -605,8 +604,6 @@ static BOOL clear_decompress_bands_data(CLEAR_CONTEXT* clear, wStream* s, UINT32
 		return FALSE;
 	}
 
-	suboffset = 0;
-
 	while (suboffset < bandsByteCount)
 	{
 		BYTE r, g, b;
@@ -742,9 +739,10 @@ static BOOL clear_decompress_bands_data(CLEAR_CONTEXT* clear, wStream* s, UINT32
 
 				for (y = 0; y < vBarShortPixelCount; y++)
 				{
-					BYTE r, g, b;
-					BYTE* dstBuffer = &vBarShortEntry->pixels[y * GetBytesPerPixel(clear->format)];
-					UINT32 color;
+					BYTE r = 0, g = 0, b = 0;
+					BYTE* dstBuffer =
+					    &vBarShortEntry->pixels[y * GetBytesPerPixel(clear->format)];
+					UINT32 color = 0;
 					Stream_Read_UINT8(s, b);
 					Stream_Read_UINT8(s, g);
 					Stream_Read_UINT8(s, r);
@@ -804,8 +802,8 @@ static BOOL clear_decompress_bands_data(CLEAR_CONTEXT* clear, wStream* s, UINT32
 
 				dstBuffer = vBarEntry->pixels;
 				/* if (y < vBarYOn), use colorBkg */
-				y = 0;
-				count = vBarYOn;
+				UINT32 y = 0;
+				UINT32 count = vBarYOn;
 
 				if ((y + count) > vBarPixelCount)
 					count = (vBarPixelCount > y) ? (vBarPixelCount - y) : 0;
@@ -874,15 +872,22 @@ static BOOL clear_decompress_bands_data(CLEAR_CONTEXT* clear, wStream* s, UINT32
 
 			if (i < nWidth)
 			{
-				count = vBarEntry->count;
+				UINT32 count = vBarEntry->count;
 
 				if (count > nHeight)
 					count = nHeight;
 
+				if (nXDstRel + i > nDstWidth)
+					return FALSE;
+
 				for (y = 0; y < count; y++)
 				{
-					BYTE* pDstPixel8 = &pDstData[((nYDstRel + y) * nDstStep) +
-					                             ((nXDstRel + i) * GetBytesPerPixel(DstFormat))];
+					if (nYDstRel + y > nDstHeight)
+						return FALSE;
+
+					BYTE* pDstPixel8 =
+					    &pDstData[((nYDstRel + y) * nDstStep) +
+					              ((nXDstRel + i) * GetBytesPerPixel(DstFormat))];
 					UINT32 color = ReadColor(pSrcPixel, clear->format);
 					color = FreeRDPConvertColor(color, clear->format, DstFormat, NULL);
 
@@ -1117,7 +1122,7 @@ INT32 clear_decompress(CLEAR_CONTEXT* clear, const BYTE* pSrcData, UINT32 SrcSiz
 	if (bandsByteCount > 0)
 	{
 		if (!clear_decompress_bands_data(clear, s, bandsByteCount, nWidth, nHeight, pDstData,
-		                                 DstFormat, nDstStep, nXDst, nYDst))
+		                                 DstFormat, nDstStep, nXDst, nYDst, nDstWidth, nDstHeight))
 		{
 			WLog_ERR(TAG, "clear_decompress_bands_data failed!");
 			goto fail;
-- 
2.42.1

From 2aaa8282d5ed493d3c505628cd76a9db41d035d9 Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Tue, 22 Aug 2023 15:05:20 +0200
Subject: [PATCH 11/15] [codec,progressive] fix missing destination checks

---
 libfreerdp/codec/progressive.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c
index e2660b19f..dc6a62087 100644
--- a/libfreerdp/codec/progressive.c
+++ b/libfreerdp/codec/progressive.c
@@ -2282,11 +2282,17 @@ INT32 progressive_decompress(PROGRESSIVE_CONTEXT* progressive, const BYTE* pSrcD
 		for (j = 0; j < nbUpdateRects; j++)
 		{
 			const RECTANGLE_16* rect = &updateRects[j];
-			const UINT32 nXSrc = rect->left - (nXDst + tile->x);
-			const UINT32 nYSrc = rect->top - (nYDst + tile->y);
+			if (rect->left < updateRect.left)
+				goto fail;
+			const UINT32 nXSrc = rect->left - updateRect.left;
+			const UINT32 nYSrc = rect->top - updateRect.top;
 			const UINT32 width = rect->right - rect->left;
 			const UINT32 height = rect->bottom - rect->top;
 
+			if (rect->left + width > surface->width)
+				goto fail;
+			if (rect->top + height > surface->height)
+				goto fail;
 			if (!freerdp_image_copy(pDstData, DstFormat, nDstStep, rect->left, rect->top, width,
 			                        height, tile->data, progressive->format, tile->stride, nXSrc,
 			                        nYSrc, NULL, FREERDP_FLIP_NONE))
-- 
2.42.1

From a312ea160031bb7d82857187c58879bb2923cf9c Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Wed, 23 Aug 2023 08:38:23 +0200
Subject: [PATCH 12/15] [primitives,yuv] clean up code, assert arguments

---
 libfreerdp/primitives/prim_YUV.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/libfreerdp/primitives/prim_YUV.c b/libfreerdp/primitives/prim_YUV.c
index d6e0d3eeb..9bee77269 100644
--- a/libfreerdp/primitives/prim_YUV.c
+++ b/libfreerdp/primitives/prim_YUV.c
@@ -25,6 +25,8 @@
 #include "config.h"
 #endif
 
+#include <winpr/assert.h>
+
 #include <freerdp/types.h>
 #include <freerdp/primitives.h>
 #include <freerdp/codec/color.h>
@@ -383,11 +385,15 @@ static pstatus_t general_YUV444ToRGB_8u_P3AC4R_general(const BYTE* const pSrc[3]
                                                        const prim_size_t* roi)
 {
 	UINT32 x, y;
-	UINT32 nWidth, nHeight;
 	const DWORD formatSize = GetBytesPerPixel(DstFormat);
 	fkt_writePixel writePixel = getPixelWriteFunction(DstFormat);
-	nWidth = roi->width;
-	nHeight = roi->height;
+
+	WINPR_ASSERT(pSrc);
+	WINPR_ASSERT(pDst);
+	WINPR_ASSERT(roi);
+
+	const UINT32 nWidth = roi->width;
+	const UINT32 nHeight = roi->height;
 
 	for (y = 0; y < nHeight; y++)
 	{
@@ -417,10 +423,14 @@ static pstatus_t general_YUV444ToRGB_8u_P3AC4R_BGRX(const BYTE* const pSrc[3],
                                                     const prim_size_t* roi)
 {
 	UINT32 x, y;
-	UINT32 nWidth, nHeight;
 	const DWORD formatSize = GetBytesPerPixel(DstFormat);
-	nWidth = roi->width;
-	nHeight = roi->height;
+
+	WINPR_ASSERT(pSrc);
+	WINPR_ASSERT(pDst);
+	WINPR_ASSERT(roi);
+
+	const UINT32 nWidth = roi->width;
+	const UINT32 nHeight = roi->height;
 
 	for (y = 0; y < nHeight; y++)
 	{
-- 
2.42.1

From 1a796ed7ac45b47eb7c0fbe00a7c5e1ad59a6fca Mon Sep 17 00:00:00 2001
From: akallabeth <akallabeth@posteo.net>
Date: Wed, 23 Aug 2023 09:24:00 +0200
Subject: [PATCH 13/15] [codec,interleaved] fix bounds checks

---
 libfreerdp/codec/include/bitmap.c | 54 +++++++++++------------------
 libfreerdp/codec/interleaved.c    | 56 +++++++++++++++++++++----------
 2 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/libfreerdp/codec/include/bitmap.c b/libfreerdp/codec/include/bitmap.c
index 38bcaa859..101e4f382 100644
--- a/libfreerdp/codec/include/bitmap.c
+++ b/libfreerdp/codec/include/bitmap.c
@@ -46,7 +46,6 @@ static INLINE BYTE* WRITEFGBGIMAGE(BYTE* pbDest, const BYTE* pbDestEnd, UINT32 r
 			data = xorPixel;
 
 		DESTWRITEPIXEL(pbDest, data);
-		DESTNEXTPIXEL(pbDest);
 		mask = mask << 1;
 	});
 	return pbDest;
@@ -76,7 +75,6 @@ static INLINE BYTE* WRITEFIRSTLINEFGBGIMAGE(BYTE* pbDest, const BYTE* pbDestEnd,
 			data = BLACK_PIXEL;
 
 		DESTWRITEPIXEL(pbDest, data);
-		DESTNEXTPIXEL(pbDest);
 		mask = mask << 1;
 	});
 	return pbDest;
@@ -134,6 +132,8 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 		if (code == REGULAR_BG_RUN || code == MEGA_MEGA_BG_RUN)
 		{
 			runLength = ExtractRunLength(code, pbSrc, &advance);
+			if (advance == 0)
+				return FALSE;
 			pbSrc = pbSrc + advance;
 
 			if (fFirstLine)
@@ -144,17 +144,13 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 						return FALSE;
 
 					DESTWRITEPIXEL(pbDest, fgPel);
-					DESTNEXTPIXEL(pbDest);
 					runLength = runLength - 1;
 				}
 
 				if (!ENSURE_CAPACITY(pbDest, pbDestEnd, runLength))
 					return FALSE;
 
-				UNROLL(runLength, {
-					DESTWRITEPIXEL(pbDest, BLACK_PIXEL);
-					DESTNEXTPIXEL(pbDest);
-				});
+				UNROLL(runLength, { DESTWRITEPIXEL(pbDest, BLACK_PIXEL); });
 			}
 			else
 			{
@@ -166,7 +162,6 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 						return FALSE;
 
 					DESTWRITEPIXEL(pbDest, temp ^ fgPel);
-					DESTNEXTPIXEL(pbDest);
 					runLength--;
 				}
 
@@ -176,7 +171,6 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 				UNROLL(runLength, {
 					DESTREADPIXEL(temp, pbDest - rowDelta);
 					DESTWRITEPIXEL(pbDest, temp);
-					DESTNEXTPIXEL(pbDest);
 				});
 			}
 
@@ -197,14 +191,15 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 			case LITE_SET_FG_FG_RUN:
 			case MEGA_MEGA_SET_FG_RUN:
 				runLength = ExtractRunLength(code, pbSrc, &advance);
+				if (advance == 0)
+					return FALSE;
 				pbSrc = pbSrc + advance;
 
 				if (code == LITE_SET_FG_FG_RUN || code == MEGA_MEGA_SET_FG_RUN)
 				{
-					if (pbSrc >= pbEnd)
+					if (pbSrc + sizeof(fgPel) >= pbEnd)
 						return FALSE;
 					SRCREADPIXEL(fgPel, pbSrc);
-					SRCNEXTPIXEL(pbSrc);
 				}
 
 				if (!ENSURE_CAPACITY(pbDest, pbDestEnd, runLength))
@@ -212,17 +207,13 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 
 				if (fFirstLine)
 				{
-					UNROLL(runLength, {
-						DESTWRITEPIXEL(pbDest, fgPel);
-						DESTNEXTPIXEL(pbDest);
-					});
+					UNROLL(runLength, { DESTWRITEPIXEL(pbDest, fgPel); });
 				}
 				else
 				{
 					UNROLL(runLength, {
 						DESTREADPIXEL(temp, pbDest - rowDelta);
 						DESTWRITEPIXEL(pbDest, temp ^ fgPel);
-						DESTNEXTPIXEL(pbDest);
 					});
 				}
 
@@ -232,24 +223,22 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 			case LITE_DITHERED_RUN:
 			case MEGA_MEGA_DITHERED_RUN:
 				runLength = ExtractRunLength(code, pbSrc, &advance);
+				if (advance == 0)
+					return FALSE;
 				pbSrc = pbSrc + advance;
-				if (pbSrc >= pbEnd)
+				if (pbSrc + sizeof(pixelA) >= pbEnd)
 					return FALSE;
 				SRCREADPIXEL(pixelA, pbSrc);
-				SRCNEXTPIXEL(pbSrc);
-				if (pbSrc >= pbEnd)
+				if (pbSrc + sizeof(pixelB) >= pbEnd)
 					return FALSE;
 				SRCREADPIXEL(pixelB, pbSrc);
-				SRCNEXTPIXEL(pbSrc);
 
 				if (!ENSURE_CAPACITY(pbDest, pbDestEnd, runLength * 2))
 					return FALSE;
 
 				UNROLL(runLength, {
 					DESTWRITEPIXEL(pbDest, pixelA);
-					DESTNEXTPIXEL(pbDest);
 					DESTWRITEPIXEL(pbDest, pixelB);
-					DESTNEXTPIXEL(pbDest);
 				});
 				break;
 
@@ -257,19 +246,17 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 			case REGULAR_COLOR_RUN:
 			case MEGA_MEGA_COLOR_RUN:
 				runLength = ExtractRunLength(code, pbSrc, &advance);
+				if (advance == 0)
+					return FALSE;
 				pbSrc = pbSrc + advance;
-				if (pbSrc >= pbEnd)
+				if (pbSrc + sizeof(pixelA) >= pbEnd)
 					return FALSE;
 				SRCREADPIXEL(pixelA, pbSrc);
-				SRCNEXTPIXEL(pbSrc);
 
 				if (!ENSURE_CAPACITY(pbDest, pbDestEnd, runLength))
 					return FALSE;
 
-				UNROLL(runLength, {
-					DESTWRITEPIXEL(pbDest, pixelA);
-					DESTNEXTPIXEL(pbDest);
-				});
+				UNROLL(runLength, { DESTWRITEPIXEL(pbDest, pixelA); });
 				break;
 
 			/* Handle Foreground/Background Image Orders. */
@@ -278,14 +265,15 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 			case LITE_SET_FG_FGBG_IMAGE:
 			case MEGA_MEGA_SET_FGBG_IMAGE:
 				runLength = ExtractRunLength(code, pbSrc, &advance);
+				if (advance == 0)
+					return FALSE;
 				pbSrc = pbSrc + advance;
 
-				if (pbSrc >= pbEnd)
+				if (!buffer_within_range(pbSrc + sizeof(fgPel), pbEnd))
 					return FALSE;
 				if (code == LITE_SET_FG_FGBG_IMAGE || code == MEGA_MEGA_SET_FGBG_IMAGE)
 				{
 					SRCREADPIXEL(fgPel, pbSrc);
-					SRCNEXTPIXEL(pbSrc);
 				}
 
 				if (fFirstLine)
@@ -343,6 +331,8 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 			case REGULAR_COLOR_IMAGE:
 			case MEGA_MEGA_COLOR_IMAGE:
 				runLength = ExtractRunLength(code, pbSrc, &advance);
+				if (advance == 0)
+					return FALSE;
 				pbSrc = pbSrc + advance;
 				if (!ENSURE_CAPACITY(pbDest, pbDestEnd, runLength))
 					return FALSE;
@@ -351,9 +341,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 					if (pbSrc >= pbEnd)
 						return FALSE;
 					SRCREADPIXEL(temp, pbSrc);
-					SRCNEXTPIXEL(pbSrc);
 					DESTWRITEPIXEL(pbDest, temp);
-					DESTNEXTPIXEL(pbDest);
 				});
 				break;
 
@@ -405,7 +393,6 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 					return FALSE;
 
 				DESTWRITEPIXEL(pbDest, WHITE_PIXEL);
-				DESTNEXTPIXEL(pbDest);
 				break;
 
 			/* Handle Black Order. */
@@ -416,7 +403,6 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 					return FALSE;
 
 				DESTWRITEPIXEL(pbDest, BLACK_PIXEL);
-				DESTNEXTPIXEL(pbDest);
 				break;
 
 			default:
diff --git a/libfreerdp/codec/interleaved.c b/libfreerdp/codec/interleaved.c
index b76fe1ca3..a5456f725 100644
--- a/libfreerdp/codec/interleaved.c
+++ b/libfreerdp/codec/interleaved.c
@@ -239,19 +239,26 @@ static INLINE void write_pixel_16(BYTE* _buf, UINT16 _pix)
 #undef DESTWRITEPIXEL
 #undef DESTREADPIXEL
 #undef SRCREADPIXEL
-#undef DESTNEXTPIXEL
-#undef SRCNEXTPIXEL
 #undef WRITEFGBGIMAGE
 #undef WRITEFIRSTLINEFGBGIMAGE
 #undef RLEDECOMPRESS
 #undef RLEEXTRA
 #undef WHITE_PIXEL
 #define WHITE_PIXEL 0xFF
-#define DESTWRITEPIXEL(_buf, _pix) write_pixel_8(_buf, _pix)
+#define DESTWRITEPIXEL(_buf, _pix) \
+	do                             \
+	{                              \
+		write_pixel_8(_buf, _pix); \
+		_buf += 1;                 \
+	} while (0)
 #define DESTREADPIXEL(_pix, _buf) _pix = (_buf)[0]
-#define SRCREADPIXEL(_pix, _buf) _pix = (_buf)[0]
-#define DESTNEXTPIXEL(_buf) _buf += 1
-#define SRCNEXTPIXEL(_buf) _buf += 1
+#define SRCREADPIXEL(_pix, _buf) \
+	do                           \
+	{                            \
+		_pix = (_buf)[0];        \
+		_buf += 1;               \
+	} while (0)
+
 #define WRITEFGBGIMAGE WriteFgBgImage8to8
 #define WRITEFIRSTLINEFGBGIMAGE WriteFirstLineFgBgImage8to8
 #define RLEDECOMPRESS RleDecompress8to8
@@ -263,19 +270,25 @@ static INLINE void write_pixel_16(BYTE* _buf, UINT16 _pix)
 #undef DESTWRITEPIXEL
 #undef DESTREADPIXEL
 #undef SRCREADPIXEL
-#undef DESTNEXTPIXEL
-#undef SRCNEXTPIXEL
 #undef WRITEFGBGIMAGE
 #undef WRITEFIRSTLINEFGBGIMAGE
 #undef RLEDECOMPRESS
 #undef RLEEXTRA
 #undef WHITE_PIXEL
 #define WHITE_PIXEL 0xFFFF
-#define DESTWRITEPIXEL(_buf, _pix) write_pixel_16(_buf, _pix)
+#define DESTWRITEPIXEL(_buf, _pix)  \
+	do                              \
+	{                               \
+		write_pixel_16(_buf, _pix); \
+		_buf += 2;                  \
+	} while (0)
 #define DESTREADPIXEL(_pix, _buf) _pix = ((UINT16*)(_buf))[0]
-#define SRCREADPIXEL(_pix, _buf) _pix = (_buf)[0] | ((_buf)[1] << 8)
-#define DESTNEXTPIXEL(_buf) _buf += 2
-#define SRCNEXTPIXEL(_buf) _buf += 2
+#define SRCREADPIXEL(_pix, _buf)             \
+	do                                       \
+	{                                        \
+		_pix = (_buf)[0] | ((_buf)[1] << 8); \
+		_buf += 2;                           \
+	} while (0)
 #define WRITEFGBGIMAGE WriteFgBgImage16to16
 #define WRITEFIRSTLINEFGBGIMAGE WriteFirstLineFgBgImage16to16
 #define RLEDECOMPRESS RleDecompress16to16
@@ -287,19 +300,26 @@ static INLINE void write_pixel_16(BYTE* _buf, UINT16 _pix)
 #undef DESTWRITEPIXEL
 #undef DESTREADPIXEL
 #undef SRCREADPIXEL
-#undef DESTNEXTPIXEL
-#undef SRCNEXTPIXEL
 #undef WRITEFGBGIMAGE
 #undef WRITEFIRSTLINEFGBGIMAGE
 #undef RLEDECOMPRESS
 #undef RLEEXTRA
 #undef WHITE_PIXEL
 #define WHITE_PIXEL 0xFFFFFF
-#define DESTWRITEPIXEL(_buf, _pix) write_pixel_24(_buf, _pix)
+#define DESTWRITEPIXEL(_buf, _pix)  \
+	do                              \
+	{                               \
+		write_pixel_24(_buf, _pix); \
+		_buf += 3;                  \
+	} while (0)
 #define DESTREADPIXEL(_pix, _buf) _pix = (_buf)[0] | ((_buf)[1] << 8) | ((_buf)[2] << 16)
-#define SRCREADPIXEL(_pix, _buf) _pix = (_buf)[0] | ((_buf)[1] << 8) | ((_buf)[2] << 16)
-#define DESTNEXTPIXEL(_buf) _buf += 3
-#define SRCNEXTPIXEL(_buf) _buf += 3
+#define SRCREADPIXEL(_pix, _buf)                                 \
+	do                                                           \
+	{                                                            \
+		_pix = (_buf)[0] | ((_buf)[1] << 8) | ((_buf)[2] << 16); \
+		_buf += 3;                                               \
+	} while (0)
+
 #define WRITEFGBGIMAGE WriteFgBgImage24to24
 #define WRITEFIRSTLINEFGBGIMAGE WriteFirstLineFgBgImage24to24
 #define RLEDECOMPRESS RleDecompress24to24
-- 
2.42.1

From 1cc160711d1b89f090da10b2534897dbfe7fd535 Mon Sep 17 00:00:00 2001
From: Youkou Tenhouin <youkou@tenhou.in>
Date: Thu, 16 Nov 2023 14:01:27 +0800
Subject: [PATCH 14/15] fixup! [codec,interleaved] fix bounds checks

---
 libfreerdp/codec/include/bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libfreerdp/codec/include/bitmap.c b/libfreerdp/codec/include/bitmap.c
index 101e4f382..355c697e0 100644
--- a/libfreerdp/codec/include/bitmap.c
+++ b/libfreerdp/codec/include/bitmap.c
@@ -269,7 +269,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY
 					return FALSE;
 				pbSrc = pbSrc + advance;
 
-				if (!buffer_within_range(pbSrc + sizeof(fgPel), pbEnd))
+				if (pbSrc + sizeof(fgPel) >= pbEnd)
 					return FALSE;
 				if (code == LITE_SET_FG_FGBG_IMAGE || code == MEGA_MEGA_SET_FGBG_IMAGE)
 				{
-- 
2.42.1

From 09939244b73b41ea12a8f6ff6c6949e75eba4982 Mon Sep 17 00:00:00 2001
From: Armin Novak <armin.novak@thincast.com>
Date: Mon, 28 Aug 2023 09:06:42 +0200
Subject: [PATCH 15/15] [codec,ncrush] fix index checks

properly verify all offsets while decoding data.
---
 libfreerdp/codec/ncrush.c | 141 ++++++++++++++++++++++++++++++--------
 1 file changed, 112 insertions(+), 29 deletions(-)

diff --git a/libfreerdp/codec/ncrush.c b/libfreerdp/codec/ncrush.c
index 3d6a216d3..545ff2a54 100644
--- a/libfreerdp/codec/ncrush.c
+++ b/libfreerdp/codec/ncrush.c
@@ -23,6 +23,7 @@
 #include "config.h"
 #endif
 
+#include <winpr/assert.h>
 #include <winpr/crt.h>
 #include <winpr/print.h>
 #include <winpr/bitstream.h>
@@ -1994,15 +1995,9 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
                       UINT32* pDstSize, UINT32 flags)
 {
 	UINT32 index;
-	UINT32 bits;
-	INT32 nbits;
-	const BYTE* SrcPtr;
-	const BYTE* SrcEnd;
-	UINT16 Mask;
 	BYTE Literal;
 	UINT32 IndexLEC;
 	UINT32 BitLength;
-	UINT32 MaskedBits;
 	UINT32 CopyOffset;
 	UINT32 CopyLength;
 	UINT32 OldCopyOffset;
@@ -2010,9 +2005,6 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 	UINT32 LengthOfMatch;
 	UINT32 CopyOffsetIndex;
 	UINT32 OffsetCacheIndex;
-	BYTE* HistoryPtr;
-	BYTE* HistoryBuffer;
-	BYTE* HistoryBufferEnd;
 	UINT32 CopyOffsetBits;
 	UINT32 CopyOffsetBase;
 	UINT32 LengthOfMatchBits;
@@ -2021,8 +2013,8 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 	if (ncrush->HistoryEndOffset != 65535)
 		return -1001;
 
-	HistoryBuffer = ncrush->HistoryBuffer;
-	HistoryBufferEnd = &HistoryBuffer[ncrush->HistoryEndOffset];
+	BYTE* HistoryBuffer = ncrush->HistoryBuffer;
+	const BYTE* HistoryBufferEnd = &HistoryBuffer[ncrush->HistoryEndOffset];
 
 	if (flags & PACKET_AT_FRONT)
 	{
@@ -2041,7 +2033,7 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 		ZeroMemory(&(ncrush->OffsetCache), sizeof(ncrush->OffsetCache));
 	}
 
-	HistoryPtr = ncrush->HistoryPtr;
+	BYTE* HistoryPtr = ncrush->HistoryPtr;
 
 	if (!(flags & PACKET_COMPRESSED))
 	{
@@ -2050,17 +2042,19 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 		return 1;
 	}
 
-	SrcEnd = &pSrcData[SrcSize];
-	nbits = 32;
-	bits = get_dword(pSrcData);
-	SrcPtr = pSrcData + 4;
+	const BYTE* SrcEnd = &pSrcData[SrcSize];
+	const BYTE* SrcPtr = pSrcData + 4;
 
+	INT32 nbits = 32;
+	UINT32 bits = get_dword(pSrcData);
 	while (1)
 	{
 		while (1)
 		{
-			Mask = get_word(&HuffTableMask[29]);
-			MaskedBits = bits & Mask;
+			const UINT16 Mask = get_word(&HuffTableMask[29]);
+			const UINT32 MaskedBits = bits & Mask;
+			if (MaskedBits >= ARRAYSIZE(HuffTableLEC))
+				return -1;
 			IndexLEC = HuffTableLEC[MaskedBits] & 0xFFF;
 			BitLength = HuffTableLEC[MaskedBits] >> 12;
 			bits >>= BitLength;
@@ -2096,8 +2090,10 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 				return -1004;
 
 			CopyOffset = ncrush->OffsetCache[OffsetCacheIndex];
-			Mask = get_word(&HuffTableMask[21]);
-			MaskedBits = bits & Mask;
+			const UINT16 Mask = get_word(&HuffTableMask[21]);
+			const UINT32 MaskedBits = bits & Mask;
+			if (MaskedBits > ARRAYSIZE(HuffTableLOM))
+				return -1;
 			LengthOfMatch = HuffTableLOM[MaskedBits] & 0xFFF;
 			BitLength = HuffTableLOM[MaskedBits] >> 12;
 			bits >>= BitLength;
@@ -2106,13 +2102,23 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 			if (!NCrushFetchBits(&SrcPtr, &SrcEnd, &nbits, &bits))
 				return -1;
 
+			if (LengthOfMatch >= ARRAYSIZE(LOMBitsLUT))
+				return -1;
+
 			LengthOfMatchBits = LOMBitsLUT[LengthOfMatch];
+
+			if (LengthOfMatch >= ARRAYSIZE(LOMBaseLUT))
+				return -1;
 			LengthOfMatchBase = LOMBaseLUT[LengthOfMatch];
 
 			if (LengthOfMatchBits)
 			{
-				Mask = get_word(&HuffTableMask[(2 * LengthOfMatchBits) + 3]);
-				MaskedBits = bits & Mask;
+				const size_t idx = (2ull * LengthOfMatchBits) + 3ull;
+				if (idx >= ARRAYSIZE(HuffTableMask))
+					return -1;
+
+				const UINT16 Mask = get_word(&HuffTableMask[idx]);
+				const UINT32 MaskedBits = bits & Mask;
 				bits >>= LengthOfMatchBits;
 				nbits -= LengthOfMatchBits;
 				LengthOfMatchBase += MaskedBits;
@@ -2127,15 +2133,28 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 		}
 		else
 		{
+			if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBitsLUT))
+				return -1;
+
 			CopyOffsetBits = CopyOffsetBitsLUT[CopyOffsetIndex];
+
+			if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBaseLUT))
+				return -1;
 			CopyOffsetBase = CopyOffsetBaseLUT[CopyOffsetIndex];
 			CopyOffset = CopyOffsetBase - 1;
 
 			if (CopyOffsetBits)
 			{
-				Mask = get_word(&HuffTableMask[(2 * CopyOffsetBits) + 3]);
-				MaskedBits = bits & Mask;
-				CopyOffset = CopyOffsetBase + MaskedBits - 1;
+				const size_t idx = (2ull * CopyOffsetBits) + 3ull;
+				if (idx >= ARRAYSIZE(HuffTableMask))
+					return -1;
+
+				const UINT16 Mask = get_word(&HuffTableMask[idx]);
+				const UINT32 MaskedBits = bits & Mask;
+				const UINT32 tmp = CopyOffsetBase + MaskedBits;
+				if (tmp < 1)
+					return -1;
+				CopyOffset = tmp - 1;
 				bits >>= CopyOffsetBits;
 				nbits -= CopyOffsetBits;
 
@@ -2143,8 +2162,11 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 					return -1;
 			}
 
-			Mask = get_word(&HuffTableMask[21]);
-			MaskedBits = bits & Mask;
+			const UINT16 Mask = get_word(&HuffTableMask[21]);
+			const UINT32 MaskedBits = bits & Mask;
+			if (MaskedBits >= ARRAYSIZE(HuffTableLOM))
+				return -1;
+
 			LengthOfMatch = HuffTableLOM[MaskedBits] & 0xFFF;
 			BitLength = HuffTableLOM[MaskedBits] >> 12;
 			bits >>= BitLength;
@@ -2153,13 +2175,23 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BY
 			if (!NCrushFetchBits(&SrcPtr, &SrcEnd, &nbits, &bits))
 				return -1;
 
+			if (LengthOfMatch >= ARRAYSIZE(LOMBitsLUT))
+				return -1;
+
 			LengthOfMatchBits = LOMBitsLUT[LengthOfMatch];
+
+			if (LengthOfMatch >= ARRAYSIZE(LOMBaseLUT))
+				return -1;
 			LengthOfMatchBase = LOMBaseLUT[LengthOfMatch];
 
 			if (LengthOfMatchBits)
 			{
-				Mask = get_word(&HuffTableMask[(2 * LengthOfMatchBits) + 3]);
-				MaskedBits = bits & Mask;
+				const size_t idx = (2ull * LengthOfMatchBits) + 3ull;
+				if (idx >= ARRAYSIZE(HuffTableMask))
+					return -1;
+
+				const UINT16 Mask = get_word(&HuffTableMask[idx]);
+				const UINT32 MaskedBits = bits & Mask;
 				bits >>= LengthOfMatchBits;
 				nbits -= LengthOfMatchBits;
 				LengthOfMatchBase += MaskedBits;
@@ -2583,7 +2615,12 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 			}
 
 			IndexLEC = Literal;
+			if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
+				return -1;
 			BitLength = HuffLengthLEC[IndexLEC];
+
+			if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
+				return -1;
 			CodeLEC = get_word(&HuffCodeLEC[IndexLEC * 2]);
 
 			if (BitLength > 15)
@@ -2666,9 +2703,18 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 					bits = CopyOffset;
 
 				CopyOffsetIndex = ncrush->HuffTableCopyOffset[bits + 2];
+
+				if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBitsLUT))
+					return -1;
+
 				CopyOffsetBits = CopyOffsetBitsLUT[CopyOffsetIndex];
 				IndexLEC = 257 + CopyOffsetIndex;
+				if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
+					return -1;
 				BitLength = HuffLengthLEC[IndexLEC];
+
+				if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
+					return -1;
 				CodeLEC = get_word(&HuffCodeLEC[IndexLEC * 2]);
 
 				if (BitLength > 15)
@@ -2687,13 +2733,23 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 				else
 					IndexCO = ncrush->HuffTableLOM[MatchLength];
 
+				if (IndexCO >= ARRAYSIZE(HuffLengthLOM))
+					return -1;
 				BitLength = HuffLengthLOM[IndexCO];
+
+				if (IndexCO >= ARRAYSIZE(LOMBitsLUT))
+					return -1;
 				IndexLOM = LOMBitsLUT[IndexCO];
+
+				if (IndexCO >= ARRAYSIZE(HuffCodeLOM))
+					return -1;
 				NCrushWriteBits(&DstPtr, &accumulator, &offset, HuffCodeLOM[IndexCO], BitLength);
 				Mask = ((1 << IndexLOM) - 1);
 				MaskedBits = (MatchLength - 2) & Mask;
 				NCrushWriteBits(&DstPtr, &accumulator, &offset, MaskedBits, IndexLOM);
 
+				if (IndexCO >= ARRAYSIZE(LOMBaseLUT))
+					return -1;
 				if ((MaskedBits + LOMBaseLUT[IndexCO]) != MatchLength)
 					return -1010;
 			}
@@ -2701,7 +2757,11 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 			{
 				/* CopyOffset in OffsetCache */
 				IndexLEC = 289 + OffsetCacheIndex;
+				if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
+					return -1;
 				BitLength = HuffLengthLEC[IndexLEC];
+				if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
+					return -1;
 				CodeLEC = get_word(&HuffCodeLEC[IndexLEC * 2]);
 
 				if (BitLength >= 15)
@@ -2714,13 +2774,24 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 				else
 					IndexCO = ncrush->HuffTableLOM[MatchLength];
 
+				if (IndexCO >= ARRAYSIZE(HuffLengthLOM))
+					return -1;
+
 				BitLength = HuffLengthLOM[IndexCO];
+
+				if (IndexCO >= ARRAYSIZE(LOMBitsLUT))
+					return -1;
 				IndexLOM = LOMBitsLUT[IndexCO];
+
+				if (IndexCO >= ARRAYSIZE(HuffCodeLOM))
+					return -1;
 				NCrushWriteBits(&DstPtr, &accumulator, &offset, HuffCodeLOM[IndexCO], BitLength);
 				Mask = ((1 << IndexLOM) - 1);
 				MaskedBits = (MatchLength - 2) & Mask;
 				NCrushWriteBits(&DstPtr, &accumulator, &offset, MaskedBits, IndexLOM);
 
+				if (IndexCO >= ARRAYSIZE(LOMBaseLUT))
+					return -1;
 				if ((MaskedBits + LOMBaseLUT[IndexCO]) != MatchLength)
 					return -1012;
 			}
@@ -2745,6 +2816,10 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, BYTE* pSrcData, UINT32 SrcSize, BYTE
 		Literal = *SrcPtr++;
 		HistoryPtr++;
 		IndexLEC = Literal;
+		if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
+			return -1;
+		if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
+			return -1;
 		BitLength = HuffLengthLEC[IndexLEC];
 		CodeLEC = get_word(&HuffCodeLEC[IndexLEC * 2]);
 
@@ -2801,6 +2876,9 @@ static int ncrush_generate_tables(NCRUSH_CONTEXT* context)
 	int j, l;
 	k = 0;
 
+	WINPR_ASSERT(context);
+	WINPR_ASSERT(28 < ARRAYSIZE(LOMBitsLUT));
+
 	for (i = 0; i < 28; i++)
 	{
 		for (j = 0; j < 1 << LOMBitsLUT[i]; j++)
@@ -2817,6 +2895,11 @@ static int ncrush_generate_tables(NCRUSH_CONTEXT* context)
 		else
 			i = context->HuffTableLOM[k];
 
+		if (i >= ARRAYSIZE(LOMBitsLUT))
+			return -1;
+		if (i >= ARRAYSIZE(LOMBaseLUT))
+			return -1;
+
 		if (((((1 << LOMBitsLUT[i]) - 1) & (k - 2)) + LOMBaseLUT[i]) != k)
 			return -1;
 	}
-- 
2.42.1

Places

File freerdp-CVE-2023-39350-to-2023-40589.patch of Package freerdp (Revision 3979d1be6a7ae3c50690615777528b21)

Places