CVE-2024-32660: Insufficient Information (NVD-CWE-noinfo)
value: FreeRDP is a free implementation of the Remote Desktop Protocol. Prior to version 3.5.1, a malicious server can crash the FreeRDP client by sending invalid huge allocation size. Version 3.5.1 contains a patch for the issue. No known workarounds are available.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-04-23T20:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/codec/zgfx.c
Commit date: 2024-04-20 21:20:44 +0200
Commit message: [codec,zgfx] allocate in segment steps

do not trust the uncompressedSize of a ZGFX_SEGMENTED_MULTIPART and
allocate the output buffer in steps after decoding a segment.

Diff: diff --git a/libfreerdp/codec/zgfx.c b/libfreerdp/codec/zgfx.c
index b7ee27511df..3a9d2e526c1 100644
--- a/libfreerdp/codec/zgfx.c
+++ b/libfreerdp/codec/zgfx.c
@@ -380,20 +380,50 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 static BYTE* aligned_zgfx_malloc(size_t size)
 {
 	return malloc(size + 64);
 }
 
+static BOOL zgfx_append(ZGFX_CONTEXT* zgfx, BYTE** ppConcatenated, size_t uncompressedSize,
+                        size_t* pUsed)
+{
+	WINPR_ASSERT(zgfx);
+	WINPR_ASSERT(ppConcatenated);
+	WINPR_ASSERT(pUsed);
+
+	const size_t used = *pUsed;
+	if (zgfx->OutputCount > UINT32_MAX - used)
+		return FALSE;
+
+	if (used + zgfx->OutputCount > uncompressedSize)
+		return FALSE;
+
+	BYTE* tmp = realloc(*ppConcatenated, used + zgfx->OutputCount + 64ull);
+	if (!tmp)
+		return FALSE;
+	*ppConcatenated = tmp;
+	CopyMemory(&tmp[used], zgfx->OutputBuffer, zgfx->OutputCount);
+	*pUsed = used + zgfx->OutputCount;
+	return TRUE;
+}
+
 int zgfx_decompress(ZGFX_CONTEXT* zgfx, const BYTE* pSrcData, UINT32 SrcSize, BYTE** ppDstData,
                     UINT32* pDstSize, UINT32 flags)
 {
 	int status = -1;
 	BYTE descriptor = 0;
 	wStream sbuffer = { 0 };
+	size_t used = 0;
+	BYTE* pConcatenated = NULL;
 	wStream* stream = Stream_StaticConstInit(&sbuffer, pSrcData, SrcSize);
 
 	WINPR_ASSERT(zgfx);
 	WINPR_ASSERT(stream);
+	WINPR_ASSERT(ppDstData);
+	WINPR_ASSERT(pDstSize);
+
+	*ppDstData = NULL;
+	*pDstSize = 0;
 
 	if (!Stream_CheckAndLogRequiredLength(TAG, stream, 1))
 		goto fail;
 
 	Stream_Read_UINT8(stream, descriptor); /* descriptor (1 byte) */
@@ -401,75 +431,62 @@ int zgfx_decompress(ZGFX_CONTEXT* zgfx, const BYTE* pSrcData, UINT32 SrcSize, BY
 	if (descriptor == ZGFX_SEGMENTED_SINGLE)
 	{
 		if (!zgfx_decompress_segment(zgfx, stream, Stream_GetRemainingLength(stream)))
 			goto fail;
 
-		*ppDstData = NULL;
-
 		if (zgfx->OutputCount > 0)
-			*ppDstData = aligned_zgfx_malloc(zgfx->OutputCount);
-
-		if (!*ppDstData)
-			goto fail;
-
-		*pDstSize = zgfx->OutputCount;
-		CopyMemory(*ppDstData, zgfx->OutputBuffer, zgfx->OutputCount);
+		{
+			if (!zgfx_append(zgfx, &pConcatenated, zgfx->OutputCount, &used))
+				goto fail;
+			if (used != zgfx->OutputCount)
+				goto fail;
+			*ppDstData = pConcatenated;
+			*pDstSize = zgfx->OutputCount;
+		}
 	}
 	else if (descriptor == ZGFX_SEGMENTED_MULTIPART)
 	{
 		UINT32 segmentSize = 0;
 		UINT16 segmentNumber = 0;
 		UINT16 segmentCount = 0;
 		UINT32 uncompressedSize = 0;
-		BYTE* pConcatenated = NULL;
-		size_t used = 0;
 
 		if (!Stream_CheckAndLogRequiredLength(TAG, stream, 6))
 			goto fail;
 
 		Stream_Read_UINT16(stream, segmentCount);     /* segmentCount (2 bytes) */
 		Stream_Read_UINT32(stream, uncompressedSize); /* uncompressedSize (4 bytes) */
 
-		if (!Stream_CheckAndLogRequiredLengthOfSize(TAG, stream, segmentCount, sizeof(UINT32)))
-			goto fail;
-
-		pConcatenated = aligned_zgfx_malloc(uncompressedSize);
-
-		if (!pConcatenated)
-			goto fail;
-
-		*ppDstData = pConcatenated;
-		*pDstSize = uncompressedSize;
-
 		for (segmentNumber = 0; segmentNumber < segmentCount; segmentNumber++)
 		{
 			if (!Stream_CheckAndLogRequiredLength(TAG, stream, sizeof(UINT32)))
 				goto fail;
 
 			Stream_Read_UINT32(stream, segmentSize); /* segmentSize (4 bytes) */
 
 			if (!zgfx_decompress_segment(zgfx, stream, segmentSize))
 				goto fail;
 
-			if (zgfx->OutputCount > UINT32_MAX - used)
+			if (!zgfx_append(zgfx, &pConcatenated, uncompressedSize, &used))
 				goto fail;
+		}
 
-			if (used + zgfx->OutputCount > uncompressedSize)
-				goto fail;
+		if (used != uncompressedSize)
+			goto fail;
 
-			CopyMemory(pConcatenated, zgfx->OutputBuffer, zgfx->OutputCount);
-			pConcatenated += zgfx->OutputCount;
-			used += zgfx->OutputCount;
-		}
+		*ppDstData = pConcatenated;
+		*pDstSize = uncompressedSize;
 	}
 	else
 	{
 		goto fail;
 	}
 
 	status = 1;
 fail:
+	if (status < 0)
+		free(pConcatenated);
 	return status;
 }
 
 static BOOL zgfx_compress_segment(ZGFX_CONTEXT* zgfx, wStream* s, const BYTE* pSrcData,
                                   UINT32 SrcSize, UINT32* pFlags)

Vulnerable code: 		*ppDstData = NULL;

			*ppDstData = aligned_zgfx_malloc(zgfx->OutputCount);

		if (!*ppDstData)
			goto fail;

		*pDstSize = zgfx->OutputCount;
		CopyMemory(*ppDstData, zgfx->OutputBuffer, zgfx->OutputCount);
		BYTE* pConcatenated = NULL;
		size_t used = 0;
		if (!Stream_CheckAndLogRequiredLengthOfSize(TAG, stream, segmentCount, sizeof(UINT32)))
			goto fail;

		pConcatenated = aligned_zgfx_malloc(uncompressedSize);

		if (!pConcatenated)
			goto fail;

		*ppDstData = pConcatenated;
		*pDstSize = uncompressedSize;

			if (zgfx->OutputCount > UINT32_MAX - used)
			if (used + zgfx->OutputCount > uncompressedSize)
				goto fail;
			CopyMemory(pConcatenated, zgfx->OutputBuffer, zgfx->OutputCount);
			pConcatenated += zgfx->OutputCount;
			used += zgfx->OutputCount;
		}
Fixed code: static BOOL zgfx_append(ZGFX_CONTEXT* zgfx, BYTE** ppConcatenated, size_t uncompressedSize,
                        size_t* pUsed)
{
	WINPR_ASSERT(zgfx);
	WINPR_ASSERT(ppConcatenated);
	WINPR_ASSERT(pUsed);

	const size_t used = *pUsed;
	if (zgfx->OutputCount > UINT32_MAX - used)
		return FALSE;

	if (used + zgfx->OutputCount > uncompressedSize)
		return FALSE;

	BYTE* tmp = realloc(*ppConcatenated, used + zgfx->OutputCount + 64ull);
	if (!tmp)
		return FALSE;
	*ppConcatenated = tmp;
	CopyMemory(&tmp[used], zgfx->OutputBuffer, zgfx->OutputCount);
	*pUsed = used + zgfx->OutputCount;
	return TRUE;
}

	size_t used = 0;
	BYTE* pConcatenated = NULL;
	WINPR_ASSERT(ppDstData);
	WINPR_ASSERT(pDstSize);

	*ppDstData = NULL;
	*pDstSize = 0;
		{
			if (!zgfx_append(zgfx, &pConcatenated, zgfx->OutputCount, &used))
				goto fail;
			if (used != zgfx->OutputCount)
				goto fail;
			*ppDstData = pConcatenated;
			*pDstSize = zgfx->OutputCount;
		}
			if (!zgfx_append(zgfx, &pConcatenated, uncompressedSize, &used))
		}
		if (used != uncompressedSize)
			goto fail;
		*ppDstData = pConcatenated;
		*pDstSize = uncompressedSize;
	if (status < 0)
		free(pConcatenated);