CVE-2018-8784: Out-of-bounds Write (CWE-787)
value: FreeRDP prior to version 2.0.0-rc4 contains a Heap-Based Buffer Overflow in function zgfx_decompress_segment() that results in a memory corruption and probably even a remote code execution.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2018-11-29T18:29Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: libfreerdp/codec/zgfx.c
Commit date: 2018-11-20 11:08:31 +0100
Commit message: Fixed CVE-2018-8784

Thanks to Eyal Itkin from Check Point Software Technologies.

Diff: diff --git a/libfreerdp/codec/zgfx.c b/libfreerdp/codec/zgfx.c
index f94c41b20..551fffc83 100644
--- a/libfreerdp/codec/zgfx.c
+++ b/libfreerdp/codec/zgfx.c
@@ -135,10 +135,11 @@ static INLINE BOOL zgfx_GetBits(ZGFX_CONTEXT* _zgfx, UINT32 _nbits)
 
 	_zgfx->cBitsRemaining -= _nbits;
 	_zgfx->cBitsCurrent -= _nbits;
 	_zgfx->bits = _zgfx->BitsCurrent >> _zgfx->cBitsCurrent;
 	_zgfx->BitsCurrent &= ((1 << _zgfx->cBitsCurrent) - 1);
+	return TRUE;
 }
 
 static void zgfx_history_buffer_ring_write(ZGFX_CONTEXT* zgfx, const BYTE* src, size_t count)
 {
 	UINT32 front;
@@ -227,23 +228,33 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 	int haveBits;
 	int inPrefix;
 	UINT32 count;
 	UINT32 distance;
 	BYTE* pbSegment;
-	size_t cbSegment = segmentSize - 1;
+	size_t cbSegment;
 
-	if ((Stream_GetRemainingLength(stream) < segmentSize) || (segmentSize < 1))
+	if (!zgfx || !stream)
+		return FALSE;
+
+	cbSegment = segmentSize - 1;
+
+	if ((Stream_GetRemainingLength(stream) < segmentSize) || (segmentSize < 1) ||
+	    (segmentSize > UINT32_MAX))
 		return FALSE;
 
 	Stream_Read_UINT8(stream, flags); /* header (1 byte) */
 	zgfx->OutputCount = 0;
 	pbSegment = Stream_Pointer(stream);
 	Stream_Seek(stream, cbSegment);
 
 	if (!(flags & PACKET_COMPRESSED))
 	{
 		zgfx_history_buffer_ring_write(zgfx, pbSegment, cbSegment);
+
+		if (cbSegment > sizeof(zgfx->OutputBuffer))
+			return FALSE;
+
 		CopyMemory(zgfx->OutputBuffer, pbSegment, cbSegment);
 		zgfx->OutputCount = cbSegment;
 		return TRUE;
 	}
 
@@ -278,10 +289,13 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 					zgfx->HistoryBuffer[zgfx->HistoryIndex] = c;
 
 					if (++zgfx->HistoryIndex == zgfx->HistoryBufferSize)
 						zgfx->HistoryIndex = 0;
 
+					if (zgfx->OutputCount >= sizeof(zgfx->OutputBuffer))
+						return FALSE;
+
 					zgfx->OutputBuffer[zgfx->OutputCount++] = c;
 				}
 				else
 				{
 					zgfx_GetBits(zgfx, ZGFX_TOKEN_TABLE[opIndex].valueBits);
@@ -311,10 +325,13 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 
 							zgfx_GetBits(zgfx, extra);
 							count += zgfx->bits;
 						}
 
+						if (count > sizeof(zgfx->OutputBuffer) - zgfx->OutputCount)
+							return FALSE;
+
 						zgfx_history_buffer_ring_read(zgfx, distance, &(zgfx->OutputBuffer[zgfx->OutputCount]), count);
 						zgfx_history_buffer_ring_write(zgfx, &(zgfx->OutputBuffer[zgfx->OutputCount]), count);
 						zgfx->OutputCount += count;
 					}
 					else
@@ -323,10 +340,14 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 						zgfx_GetBits(zgfx, 15);
 						count = zgfx->bits;
 						zgfx->cBitsRemaining -= zgfx->cBitsCurrent;
 						zgfx->cBitsCurrent = 0;
 						zgfx->BitsCurrent = 0;
+
+						if (count > sizeof(zgfx->OutputBuffer) - zgfx->OutputCount)
+							return FALSE;
+
 						CopyMemory(&(zgfx->OutputBuffer[zgfx->OutputCount]), zgfx->pbInputCurrent, count);
 						zgfx_history_buffer_ring_write(zgfx, zgfx->pbInputCurrent, count);
 						zgfx->pbInputCurrent += count;
 						zgfx->cBitsRemaining -= (8 * count);
 						zgfx->OutputCount += count;

Vulnerable code: 	size_t cbSegment = segmentSize - 1;
	if ((Stream_GetRemainingLength(stream) < segmentSize) || (segmentSize < 1))
Fixed code: 	return TRUE;
	size_t cbSegment;
	if (!zgfx || !stream)
		return FALSE;

	cbSegment = segmentSize - 1;

	if ((Stream_GetRemainingLength(stream) < segmentSize) || (segmentSize < 1) ||
	    (segmentSize > UINT32_MAX))

		if (cbSegment > sizeof(zgfx->OutputBuffer))
			return FALSE;

					if (zgfx->OutputCount >= sizeof(zgfx->OutputBuffer))
						return FALSE;

						if (count > sizeof(zgfx->OutputBuffer) - zgfx->OutputCount)
							return FALSE;


						if (count > sizeof(zgfx->OutputBuffer) - zgfx->OutputCount)
							return FALSE;