Vulnerability: 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.
Vulnerability ID: CVE-2018-8784
CWE: CWE-787
Severity score: 6.4
Project: https://github.com/FreeRDP/FreeRDP
Published: 2018-11-29
File: libfreerdp/codec/zgfx.c
Commit message: Fixed CVE-2018-8784

Thanks to Eyal Itkin from Check Point Software Technologies.
Commit author: Armin Novak

Vulnerable version (before fix):
 static INLINE BOOL zgfx_GetBits(ZGFX_CONTEXT* _zgfx, UINT32 _nbits) 	_zgfx->cBitsCurrent -= _nbits; 	_zgfx->bits = _zgfx->BitsCurrent >> _zgfx->cBitsCurrent; 	_zgfx->BitsCurrent &= ((1 << _zgfx->cBitsCurrent) - 1); }  static void zgfx_history_buffer_ring_write(ZGFX_CONTEXT* zgfx, const BYTE* src, size_t count) static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 	UINT32 count; 	UINT32 distance; 	BYTE* pbSegment; 	size_t cbSegment = segmentSize - 1;  	if ((Stream_GetRemainingLength(stream) < segmentSize) || (segmentSize < 1)) 		return FALSE;  	Stream_Read_UINT8(stream, flags); /* header (1 byte) */ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 	if (!(flags & PACKET_COMPRESSED)) 	{ 		zgfx_history_buffer_ring_write(zgfx, pbSegment, cbSegment); 		CopyMemory(zgfx->OutputBuffer, pbSegment, cbSegment); 		zgfx->OutputCount = cbSegment; 		return TRUE; static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 					if (++zgfx->HistoryIndex == zgfx->HistoryBufferSize) 						zgfx->HistoryIndex = 0;  					zgfx->OutputBuffer[zgfx->OutputCount++] = c; 				} 				else static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 							count += zgfx->bits; 						}  						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; static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 						zgfx->cBitsRemaining -= zgfx->cBitsCurrent; 						zgfx->cBitsCurrent = 0; 						zgfx->BitsCurrent = 0; 						CopyMemory(&(zgfx->OutputBuffer[zgfx->OutputCount]), zgfx->pbInputCurrent, count); 						zgfx_history_buffer_ring_write(zgfx, zgfx->pbInputCurrent, count); 						zgfx->pbInputCurrent += count;

Fixed version (after fix):
 static INLINE BOOL zgfx_GetBits(ZGFX_CONTEXT* _zgfx, UINT32 _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) static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 	UINT32 count; 	UINT32 distance; 	BYTE* pbSegment; 	size_t cbSegment;  	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) */ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 	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; static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 					if (++zgfx->HistoryIndex == zgfx->HistoryBufferSize) 						zgfx->HistoryIndex = 0;  					if (zgfx->OutputCount >= sizeof(zgfx->OutputBuffer)) 						return FALSE;  					zgfx->OutputBuffer[zgfx->OutputCount++] = c; 				} 				else static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 							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; static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t 						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;

Patch:
patch: @@ -137,6 +137,7 @@ static INLINE BOOL zgfx_GetBits(ZGFX_CONTEXT* _zgfx, UINT32 _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)
@@ -229,9 +230,15 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 	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) */
@@ -242,6 +249,10 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 	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;
@@ -280,6 +291,9 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 					if (++zgfx->HistoryIndex == zgfx->HistoryBufferSize)
 						zgfx->HistoryIndex = 0;
 
+					if (zgfx->OutputCount >= sizeof(zgfx->OutputBuffer))
+						return FALSE;
+
 					zgfx->OutputBuffer[zgfx->OutputCount++] = c;
 				}
 				else
@@ -313,6 +327,9 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 							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;
@@ -325,6 +342,10 @@ static BOOL zgfx_decompress_segment(ZGFX_CONTEXT* zgfx, wStream* stream, size_t
 						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;
path: libfreerdp/codec/zgfx.c