CVE-2023-39354: Out-of-bounds Read (CWE-125)
value: FreeRDP is a free implementation of the Remote Desktop Protocol (RDP), released under the Apache license. Affected versions are subject to an Out-Of-Bounds Read in the `nsc_rle_decompress_data` function. The Out-Of-Bounds Read occurs because it processes `context->Planes` without  checking if it contains data of sufficient length. Should an attacker be able to leverage this vulnerability they may be able to cause a crash. This issue has been addressed in versions 2.11.0 and 3.0.0-beta3. Users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 7.5
Published: 2023-08-31T20:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/codec/nsc.c, libfreerdp/codec/nsc_types.h
Commit date: 2023-08-28 09:13:59 +0200
Commit message: [codec,nsc] fix input length validation

Diff: diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c
index 5884fe684e4..5a505607519 100644
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -110,16 +110,21 @@ 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;
 
 		if (left == 5)
 		{
@@ -128,21 +133,30 @@ static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalS
 
 			outSize--;
 			*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;
 				len |= ((UINT32)(*in++)) << 16U;
 				len |= ((UINT32)(*in++)) << 24U;
@@ -168,45 +182,54 @@ 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;
 }
 
 static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 {
 	if (!context)
 		return FALSE;
 
-	BYTE* rle = context->Planes;
+	const BYTE* rle = context->Planes;
+	size_t rleSize = context->PlanesSize;
 	WINPR_ASSERT(rle);
 
 	for (size_t i = 0; i < 4; i++)
 	{
 		const UINT32 originalSize = context->OrgByteCount[i];
 		const UINT32 planeSize = context->PlaneByteCount[i];
 
+		if (rleSize < planeSize)
+			return FALSE;
+
 		if (planeSize == 0)
 		{
 			if (context->priv->PlaneBuffersLength < originalSize)
 				return FALSE;
 
 			FillMemory(context->priv->PlaneBuffers[i], originalSize, 0xFF);
 		}
 		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;
 		}
 		else
 		{
 			if (context->priv->PlaneBuffersLength < originalSize)
 				return FALSE;
 
+			if (rleSize < originalSize)
+				return FALSE;
+
 			CopyMemory(context->priv->PlaneBuffers[i], rle, originalSize);
 		}
 
 		rle += planeSize;
 	}
@@ -230,10 +253,11 @@ static BOOL nsc_stream_initialize(NSC_CONTEXT* context, wStream* s)
 
 	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);
 }
 
 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 eaf73895ea7..731a08af542 100644
--- a/libfreerdp/codec/nsc_types.h
+++ b/libfreerdp/codec/nsc_types.h
@@ -55,10 +55,11 @@ struct S_NSC_CONTEXT
 	UINT16 height;
 	BYTE* BitmapData;
 	UINT32 BitmapDataLength;
 
 	BYTE* Planes;
+	size_t PlanesSize;
 	UINT32 PlaneByteCount[4];
 	UINT32 ColorLossLevel;
 	UINT32 ChromaSubsamplingLevel;
 	BOOL DynamicColorFidelity;
 

Vulnerable code: static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalSize)
			if (*in < 0xFF)
	BYTE* rle = context->Planes;
			if (!nsc_rle_decode(rle, context->priv->PlaneBuffers[i],
Fixed code: static BOOL nsc_rle_decode(const BYTE* in, size_t inSize, BYTE* out, UINT32 outSize,
                           UINT32 originalSize)
		if (inSize < 1)
			return FALSE;
		inSize--;

		else if (inSize < 1)
			return FALSE;
			inSize--;
			if (inSize < 1)
				return FALSE;
			else if (*in < 0xFF)
				inSize--;
				if (inSize < 5)
					return FALSE;
				inSize -= 5;
	if (inSize < 4)
		return FALSE;
	const BYTE* rle = context->Planes;
	size_t rleSize = context->PlanesSize;
		if (rleSize < planeSize)
			return FALSE;

			if (!nsc_rle_decode(rle, rleSize, context->priv->PlaneBuffers[i],
			if (rleSize < originalSize)
				return FALSE;

	context->PlanesSize = total;
	size_t PlanesSize;