CVE-2023-40589: Buffer Copy without Checking Size of Input ('Classic Buffer Overflow') (CWE-120)
value: FreeRDP is a free implementation of the Remote Desktop Protocol (RDP), released under the Apache license. In affected versions there is a Global-Buffer-Overflow in the ncrush_decompress function. Feeding crafted input into this function can trigger the overflow which has only been shown 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 issue.
CWE description: The product copies an input buffer to an output buffer without verifying that the size of the input buffer is less than the size of the output buffer, leading to a buffer overflow.
CVSS v3 base score: 7.5
Published: 2023-08-31T19:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/codec/ncrush.c
Commit date: 2023-08-28 13:35:45 +0200
Commit message: [codec,ncrush] fix index checks

properly verify all offsets while decoding data.

Diff: diff --git a/libfreerdp/codec/ncrush.c b/libfreerdp/codec/ncrush.c
index eb7fc093f3f..9f0985478a9 100644
--- a/libfreerdp/codec/ncrush.c
+++ b/libfreerdp/codec/ncrush.c
@@ -2014,29 +2014,20 @@ static INLINE void NCrushWriteFinish(BYTE** DstPtr, UINT32 accumulator)
 
 int ncrush_decompress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize,
                       const BYTE** ppDstData, 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;
 	BYTE* CopyOffsetPtr;
 	UINT32 LengthOfMatch;
 	UINT32 CopyOffsetIndex;
 	UINT32 OffsetCacheIndex;
-	BYTE* HistoryPtr;
-	BYTE* HistoryBuffer;
-	BYTE* HistoryBufferEnd;
 	UINT32 CopyOffsetBits;
 	UINT32 CopyOffsetBase;
 	UINT32 LengthOfMatchBits;
 	UINT32 LengthOfMatchBase;
 
@@ -2046,12 +2037,12 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSi
 	WINPR_ASSERT(pDstSize);
 
 	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)
 	{
 		if ((ncrush->HistoryPtr - 32768) <= HistoryBuffer)
 			return -1002;
@@ -2066,30 +2057,32 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSi
 		ncrush->HistoryPtr = HistoryBuffer;
 		ZeroMemory(HistoryBuffer, sizeof(ncrush->HistoryBuffer));
 		ZeroMemory(&(ncrush->OffsetCache), sizeof(ncrush->OffsetCache));
 	}
 
-	HistoryPtr = ncrush->HistoryPtr;
+	BYTE* HistoryPtr = ncrush->HistoryPtr;
 
 	if (!(flags & PACKET_COMPRESSED))
 	{
 		*ppDstData = pSrcData;
 		*pDstSize = SrcSize;
 		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;
 			nbits -= BitLength;
 
@@ -2121,27 +2114,39 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSi
 
 			if (OffsetCacheIndex >= 4)
 				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;
 			nbits -= BitLength;
 
 			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;
 
 				if (!NCrushFetchBits(&SrcPtr, &SrcEnd, &nbits, &bits))
@@ -2152,43 +2157,69 @@ int ncrush_decompress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSi
 			ncrush->OffsetCache[OffsetCacheIndex] = ncrush->OffsetCache[0];
 			ncrush->OffsetCache[0] = OldCopyOffset;
 		}
 		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;
 
 				if (!NCrushFetchBits(&SrcPtr, &SrcEnd, &nbits, &bits))
 					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;
 			nbits -= BitLength;
 
 			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;
 
 				if (!NCrushFetchBits(&SrcPtr, &SrcEnd, &nbits, &bits))
@@ -2642,11 +2673,16 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize
 				*pDstSize = SrcSize;
 				return 1;
 			}
 
 			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)
 				return -1006;
 
@@ -2725,13 +2761,22 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize
 					bits = (CopyOffset >> 7) + 256;
 				else
 					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)
 					return -1008;
 
@@ -2746,25 +2791,39 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize
 				if ((MatchLength - 2) >= 768)
 					IndexCO = 28;
 				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;
 			}
 			else
 			{
 				/* 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)
 					return -1011;
 
@@ -2773,17 +2832,28 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize
 				if ((MatchLength - 2) >= 768)
 					IndexCO = 28;
 				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;
 			}
 		}
 
@@ -2804,10 +2874,14 @@ int ncrush_compress(NCRUSH_CONTEXT* ncrush, const BYTE* pSrcData, UINT32 SrcSize
 		}
 
 		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]);
 
 		if (BitLength > 15)
 			return -1014;
@@ -2864,10 +2938,11 @@ static int ncrush_generate_tables(NCRUSH_CONTEXT* context)
 	UINT32 k, i;
 	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++)
 		{
@@ -2881,10 +2956,15 @@ static int ncrush_generate_tables(NCRUSH_CONTEXT* context)
 		if ((k - 2) >= 768)
 			i = 28;
 		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;
 	}
 
 	k = 0;

Vulnerable code: 	UINT32 bits;
	INT32 nbits;
	const BYTE* SrcPtr;
	const BYTE* SrcEnd;
	UINT16 Mask;
	UINT32 MaskedBits;
	BYTE* HistoryPtr;
	BYTE* HistoryBuffer;
	BYTE* HistoryBufferEnd;
	HistoryBuffer = ncrush->HistoryBuffer;
	HistoryBufferEnd = &HistoryBuffer[ncrush->HistoryEndOffset];
	HistoryPtr = ncrush->HistoryPtr;
	SrcEnd = &pSrcData[SrcSize];
	nbits = 32;
	bits = get_dword(pSrcData);
	SrcPtr = pSrcData + 4;
			Mask = get_word(&HuffTableMask[29]);
			MaskedBits = bits & Mask;
			Mask = get_word(&HuffTableMask[21]);
			MaskedBits = bits & Mask;
				Mask = get_word(&HuffTableMask[(2 * LengthOfMatchBits) + 3]);
				MaskedBits = bits & Mask;
				Mask = get_word(&HuffTableMask[(2 * CopyOffsetBits) + 3]);
				MaskedBits = bits & Mask;
				CopyOffset = CopyOffsetBase + MaskedBits - 1;
			Mask = get_word(&HuffTableMask[21]);
			MaskedBits = bits & Mask;
				Mask = get_word(&HuffTableMask[(2 * LengthOfMatchBits) + 3]);
				MaskedBits = bits & Mask;
Fixed code: 	BYTE* HistoryBuffer = ncrush->HistoryBuffer;
	const BYTE* HistoryBufferEnd = &HistoryBuffer[ncrush->HistoryEndOffset];
	BYTE* HistoryPtr = ncrush->HistoryPtr;
	const BYTE* SrcEnd = &pSrcData[SrcSize];
	const BYTE* SrcPtr = pSrcData + 4;
	INT32 nbits = 32;
	UINT32 bits = get_dword(pSrcData);
			const UINT16 Mask = get_word(&HuffTableMask[29]);
			const UINT32 MaskedBits = bits & Mask;
			if (MaskedBits >= ARRAYSIZE(HuffTableLEC))
				return -1;
			const UINT16 Mask = get_word(&HuffTableMask[21]);
			const UINT32 MaskedBits = bits & Mask;
			if (MaskedBits > ARRAYSIZE(HuffTableLOM))
				return -1;
			if (LengthOfMatch >= ARRAYSIZE(LOMBitsLUT))
				return -1;


			if (LengthOfMatch >= ARRAYSIZE(LOMBaseLUT))
				return -1;
				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;
			if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBitsLUT))
				return -1;


			if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBaseLUT))
				return -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;
			const UINT16 Mask = get_word(&HuffTableMask[21]);
			const UINT32 MaskedBits = bits & Mask;
			if (MaskedBits >= ARRAYSIZE(HuffTableLOM))
				return -1;

			if (LengthOfMatch >= ARRAYSIZE(LOMBitsLUT))
				return -1;


			if (LengthOfMatch >= ARRAYSIZE(LOMBaseLUT))
				return -1;
				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;
			if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
				return -1;

			if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
				return -1;

				if (CopyOffsetIndex >= ARRAYSIZE(CopyOffsetBitsLUT))
					return -1;

				if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
					return -1;

				if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
					return -1;
				if (IndexCO >= ARRAYSIZE(HuffLengthLOM))
					return -1;

				if (IndexCO >= ARRAYSIZE(LOMBitsLUT))
					return -1;

				if (IndexCO >= ARRAYSIZE(HuffCodeLOM))
					return -1;
				if (IndexCO >= ARRAYSIZE(LOMBaseLUT))
					return -1;
				if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
					return -1;
				if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
					return -1;
				if (IndexCO >= ARRAYSIZE(HuffLengthLOM))
					return -1;


				if (IndexCO >= ARRAYSIZE(LOMBitsLUT))
					return -1;

				if (IndexCO >= ARRAYSIZE(HuffCodeLOM))
					return -1;
				if (IndexCO >= ARRAYSIZE(LOMBaseLUT))
					return -1;
		if (IndexLEC >= ARRAYSIZE(HuffLengthLEC))
			return -1;
		if (IndexLEC * 2ull >= ARRAYSIZE(HuffCodeLEC))
			return -1;
	WINPR_ASSERT(28 < ARRAYSIZE(LOMBitsLUT));
		if (i >= ARRAYSIZE(LOMBitsLUT))
			return -1;
		if (i >= ARRAYSIZE(LOMBaseLUT))
			return -1;