CVE-2018-8788: Out-of-bounds Write (CWE-787)
value: FreeRDP prior to version 2.0.0-rc4 contains an Out-Of-Bounds Write of up to 4 bytes in function nsc_rle_decode() that results in a memory corruption and possibly 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: include/freerdp/codec/nsc.h, libfreerdp/codec/nsc.c, libfreerdp/codec/nsc_encode.c, libfreerdp/codec/nsc_encode.h, libfreerdp/codec/nsc_sse2.c
Commit date: 2018-11-20 11:08:31 +0100
Commit message: Fixed CVE-2018-8788

Thanks to Eyal Itkin from Check Point Software Technologies.

Diff: diff --git a/include/freerdp/codec/nsc.h b/include/freerdp/codec/nsc.h
index 21e575ad2..cb16570db 100644
--- a/include/freerdp/codec/nsc.h
+++ b/include/freerdp/codec/nsc.h
@@ -75,12 +75,12 @@ struct _NSC_CONTEXT
 	BOOL DynamicColorFidelity;
 
 	/* color palette allocated by the application */
 	const BYTE* palette;
 
-	void (*decode)(NSC_CONTEXT* context);
-	void (*encode)(NSC_CONTEXT* context, const BYTE* BitmapData,
+	BOOL (*decode)(NSC_CONTEXT* context);
+	BOOL (*encode)(NSC_CONTEXT* context, const BYTE* BitmapData,
 	               UINT32 rowstride);
 
 	NSC_CONTEXT_PRIV* priv;
 };
 
diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c
index d0547cd33..bad0aec1c 100644
--- a/libfreerdp/codec/nsc.c
+++ b/libfreerdp/codec/nsc.c
@@ -40,17 +40,28 @@
 
 #ifndef NSC_INIT_SIMD
 #define NSC_INIT_SIMD(_nsc_context) do { } while (0)
 #endif
 
-static void nsc_decode(NSC_CONTEXT* context)
+static BOOL nsc_decode(NSC_CONTEXT* context)
 {
 	UINT16 x;
 	UINT16 y;
-	UINT16 rw = ROUND_UP_TO(context->width, 8);
-	BYTE shift = context->ColorLossLevel - 1; /* colorloss recovery + YCoCg shift */
-	BYTE* bmpdata = context->BitmapData;
+	UINT16 rw;
+	BYTE shift;
+	BYTE* bmpdata;
+	size_t pos = 0;
+
+	if (!context)
+		return FALSE;
+
+	rw = ROUND_UP_TO(context->width, 8);
+	shift = context->ColorLossLevel - 1; /* colorloss recovery + YCoCg shift */
+	bmpdata = context->BitmapData;
+
+	if (!bmpdata)
+		return FALSE;
 
 	for (y = 0; y < context->height; y++)
 	{
 		const BYTE* yplane;
 		const BYTE* coplane;
@@ -78,23 +89,30 @@ static void nsc_decode(NSC_CONTEXT* context)
 			INT16 co_val = (INT16)(INT8)(*coplane << shift);
 			INT16 cg_val = (INT16)(INT8)(*cgplane << shift);
 			INT16 r_val = y_val + co_val - cg_val;
 			INT16 g_val = y_val + cg_val;
 			INT16 b_val = y_val - co_val - cg_val;
+
+			if (pos + 4 > context->BitmapDataLength)
+				return FALSE;
+
+			pos += 4;
 			*bmpdata++ = MINMAX(b_val, 0, 0xFF);
 			*bmpdata++ = MINMAX(g_val, 0, 0xFF);
 			*bmpdata++ = MINMAX(r_val, 0, 0xFF);
 			*bmpdata++ = *aplane;
 			yplane++;
 			coplane += (context->ChromaSubsamplingLevel ? x % 2 : 1);
 			cgplane += (context->ChromaSubsamplingLevel ? x % 2 : 1);
 			aplane++;
 		}
 	}
+
+	return TRUE;
 }
 
-static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize)
+static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalSize)
 {
 	UINT32 len;
 	UINT32 left;
 	BYTE value;
 	left = originalSize;
@@ -103,10 +121,14 @@ static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize)
 	{
 		value = *in++;
 
 		if (left == 5)
 		{
+			if (outSize < 1)
+				return FALSE;
+
+			outSize--;
 			*out++ = value;
 			left--;
 		}
 		else if (value == *in)
 		{
@@ -122,46 +144,78 @@ static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize)
 				in++;
 				len = *((UINT32*) in);
 				in += 4;
 			}
 
+			if (outSize < len)
+				return FALSE;
+
+			outSize -= len;
 			FillMemory(out, len, value);
 			out += len;
 			left -= len;
 		}
 		else
 		{
+			if (outSize < 1)
+				return FALSE;
+
+			outSize--;
 			*out++ = value;
 			left--;
 		}
 	}
 
-	*((UINT32*)out) = *((UINT32*)in);
+	if ((outSize < 4) || (left < 4))
+		return FALSE;
+
+	memcpy(out, in, 4);
+	return TRUE;
 }
 
-static void nsc_rle_decompress_data(NSC_CONTEXT* context)
+static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)
 {
 	UINT16 i;
 	BYTE* rle;
 	UINT32 planeSize;
 	UINT32 originalSize;
+
+	if (!context)
+		return FALSE;
+
 	rle = context->Planes;
 
 	for (i = 0; i < 4; i++)
 	{
 		originalSize = context->OrgByteCount[i];
 		planeSize = context->PlaneByteCount[i];
 
 		if (planeSize == 0)
+		{
+			if (context->priv->PlaneBuffersLength < originalSize)
+				return FALSE;
+
 			FillMemory(context->priv->PlaneBuffers[i], originalSize, 0xFF);
+		}
 		else if (planeSize < originalSize)
-			nsc_rle_decode(rle, context->priv->PlaneBuffers[i], originalSize);
+		{
+			if (!nsc_rle_decode(rle, context->priv->PlaneBuffers[i], context->priv->PlaneBuffersLength,
+			                    originalSize))
+				return FALSE;
+		}
 		else
+		{
+			if (context->priv->PlaneBuffersLength < originalSize)
+				return FALSE;
+
 			CopyMemory(context->priv->PlaneBuffers[i], rle, originalSize);
+		}
 
 		rle += planeSize;
 	}
+
+	return TRUE;
 }
 
 static BOOL nsc_stream_initialize(NSC_CONTEXT* context, wStream* s)
 {
 	int i;
@@ -394,17 +448,29 @@ BOOL nsc_process_message(NSC_CONTEXT* context, UINT16 bpp,
 
 	if (!ret)
 		return FALSE;
 
 	/* RLE decode */
-	PROFILER_ENTER(context->priv->prof_nsc_rle_decompress_data)
-	nsc_rle_decompress_data(context);
-	PROFILER_EXIT(context->priv->prof_nsc_rle_decompress_data)
+	{
+		BOOL rc;
+		PROFILER_ENTER(context->priv->prof_nsc_rle_decompress_data)
+		rc = nsc_rle_decompress_data(context);
+		PROFILER_EXIT(context->priv->prof_nsc_rle_decompress_data)
+
+		if (!rc)
+			return FALSE;
+	}
 	/* Colorloss recover, Chroma supersample and AYCoCg to ARGB Conversion in one step */
-	PROFILER_ENTER(context->priv->prof_nsc_decode)
-	context->decode(context);
-	PROFILER_EXIT(context->priv->prof_nsc_decode)
+	{
+		BOOL rc;
+		PROFILER_ENTER(context->priv->prof_nsc_decode)
+		rc = context->decode(context);
+		PROFILER_EXIT(context->priv->prof_nsc_decode)
+
+		if (!rc)
+			return FALSE;
+	}
 
 	if (!freerdp_image_copy(pDstData, DstFormat, nDstStride, nXDst, nYDst,
 	                        width, height, context->BitmapData,
 	                        PIXEL_FORMAT_BGRA32, 0, 0, 0, NULL, flip))
 		return FALSE;
diff --git a/libfreerdp/codec/nsc_encode.c b/libfreerdp/codec/nsc_encode.c
index 492f170dc..d2456fb93 100644
--- a/libfreerdp/codec/nsc_encode.c
+++ b/libfreerdp/codec/nsc_encode.c
@@ -49,10 +49,11 @@ static BOOL nsc_context_initialize_encode(NSC_CONTEXT* context)
 	if (length > context->priv->PlaneBuffersLength)
 	{
 		for (i = 0; i < 5; i++)
 		{
 			BYTE* tmp = (BYTE*) realloc(context->priv->PlaneBuffers[i], length);
+
 			if (!tmp)
 				goto fail;
 
 			context->priv->PlaneBuffers[i] = tmp;
 		}
@@ -85,11 +86,11 @@ fail:
 	}
 
 	return FALSE;
 }
 
-static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,
+static BOOL nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,
                                       UINT32 scanline)
 {
 	UINT16 x;
 	UINT16 y;
 	UINT16 rw;
@@ -102,14 +103,24 @@ static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,
 	INT16 r_val;
 	INT16 g_val;
 	INT16 b_val;
 	BYTE a_val;
 	UINT32 tempWidth;
+
+	if (!context || data || (scanline == 0))
+		return FALSE;
+
 	tempWidth = ROUND_UP_TO(context->width, 8);
 	rw = (context->ChromaSubsamplingLevel ? tempWidth : context->width);
 	ccl = context->ColorLossLevel;
 
+	if (context->priv->PlaneBuffersLength < rw * scanline)
+		return FALSE;
+
+	if (rw < scanline * 2)
+		return FALSE;
+
 	for (y = 0; y < context->height; y++)
 	{
 		src = data + (context->height - 1 - y) * scanline;
 		yplane = context->priv->PlaneBuffers[0] + y * rw;
 		coplane = context->priv->PlaneBuffers[1] + y * rw;
@@ -240,35 +251,41 @@ static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,
 		cgplane = context->priv->PlaneBuffers[2] + y * rw;
 		CopyMemory(yplane, yplane - rw, rw);
 		CopyMemory(coplane, coplane - rw, rw);
 		CopyMemory(cgplane, cgplane - rw, rw);
 	}
+
+	return TRUE;
 }
 
-static void nsc_encode_subsampling(NSC_CONTEXT* context)
+static BOOL nsc_encode_subsampling(NSC_CONTEXT* context)
 {
 	UINT16 x;
 	UINT16 y;
-	BYTE* co_dst;
-	BYTE* cg_dst;
-	INT8* co_src0;
-	INT8* co_src1;
-	INT8* cg_src0;
-	INT8* cg_src1;
 	UINT32 tempWidth;
 	UINT32 tempHeight;
+
+	if (!context)
+		return FALSE;
+
 	tempWidth = ROUND_UP_TO(context->width, 8);
 	tempHeight = ROUND_UP_TO(context->height, 2);
 
+	if (tempHeight == 0)
+		return FALSE;
+
+	if (tempWidth > context->priv->PlaneBuffersLength / tempHeight)
+		return FALSE;
+
 	for (y = 0; y < tempHeight >> 1; y++)
 	{
-		co_dst = context->priv->PlaneBuffers[1] + y * (tempWidth >> 1);
-		cg_dst = context->priv->PlaneBuffers[2] + y * (tempWidth >> 1);
-		co_src0 = (INT8*) context->priv->PlaneBuffers[1] + (y << 1) * tempWidth;
-		co_src1 = co_src0 + tempWidth;
-		cg_src0 = (INT8*) context->priv->PlaneBuffers[2] + (y << 1) * tempWidth;
-		cg_src1 = cg_src0 + tempWidth;
+		BYTE* co_dst = context->priv->PlaneBuffers[1] + y * (tempWidth >> 1);
+		BYTE* cg_dst = context->priv->PlaneBuffers[2] + y * (tempWidth >> 1);
+		const INT8* co_src0 = (INT8*) context->priv->PlaneBuffers[1] + (y << 1) * tempWidth;
+		const INT8* co_src1 = co_src0 + tempWidth;
+		const INT8* cg_src0 = (INT8*) context->priv->PlaneBuffers[2] + (y << 1) * tempWidth;
+		const INT8* cg_src1 = cg_src0 + tempWidth;
 
 		for (x = 0; x < tempWidth >> 1; x++)
 		{
 			*co_dst++ = (BYTE)(((INT16) * co_src0 + (INT16) * (co_src0 + 1) +
 			                    (INT16) * co_src1 + (INT16) * (co_src1 + 1)) >> 2);
@@ -278,23 +295,32 @@ static void nsc_encode_subsampling(NSC_CONTEXT* context)
 			co_src1 += 2;
 			cg_src0 += 2;
 			cg_src1 += 2;
 		}
 	}
+
+	return TRUE;
 }
 
-void nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata, UINT32 rowstride)
+BOOL nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata, UINT32 rowstride)
 {
-	nsc_encode_argb_to_aycocg(context, bmpdata, rowstride);
+	if (!context || !bmpdata || (rowstride == 0))
+		return FALSE;
+
+	if (!nsc_encode_argb_to_aycocg(context, bmpdata, rowstride))
+		return FALSE;
 
 	if (context->ChromaSubsamplingLevel)
 	{
-		nsc_encode_subsampling(context);
+		if (!nsc_encode_subsampling(context))
+			return FALSE;
 	}
+
+	return TRUE;
 }
 
-static UINT32 nsc_rle_encode(BYTE* in, BYTE* out, UINT32 originalSize)
+static UINT32 nsc_rle_encode(const BYTE* in, BYTE* out, UINT32 originalSize)
 {
 	UINT32 left;
 	UINT32 runlength = 1;
 	UINT32 planeSize = 0;
 	left = originalSize;
diff --git a/libfreerdp/codec/nsc_encode.h b/libfreerdp/codec/nsc_encode.h
index e220de407..784ccb6e3 100644
--- a/libfreerdp/codec/nsc_encode.h
+++ b/libfreerdp/codec/nsc_encode.h
@@ -22,9 +22,9 @@
 #ifndef FREERDP_LIB_CODEC_NSC_ENCODE_H
 #define FREERDP_LIB_CODEC_NSC_ENCODE_H
 
 #include <freerdp/api.h>
 
-FREERDP_LOCAL void nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata,
+FREERDP_LOCAL BOOL nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata,
                               UINT32 rowstride);
 
 #endif /* FREERDP_LIB_CODEC_NSC_ENCODE_H */
diff --git a/libfreerdp/codec/nsc_sse2.c b/libfreerdp/codec/nsc_sse2.c
index 149e80a83..966525f16 100644
--- a/libfreerdp/codec/nsc_sse2.c
+++ b/libfreerdp/codec/nsc_sse2.c
@@ -383,19 +383,21 @@ static void nsc_encode_subsampling_sse2(NSC_CONTEXT* context)
 			cg_src1 += 16;
 		}
 	}
 }
 
-static void nsc_encode_sse2(NSC_CONTEXT* context, const BYTE* data,
+static BOOL nsc_encode_sse2(NSC_CONTEXT* context, const BYTE* data,
                             UINT32 scanline)
 {
 	nsc_encode_argb_to_aycocg_sse2(context, data, scanline);
 
 	if (context->ChromaSubsamplingLevel > 0)
 	{
 		nsc_encode_subsampling_sse2(context);
 	}
+
+	return TRUE;
 }
 
 void nsc_init_sse2(NSC_CONTEXT* context)
 {
 	if (!IsProcessorFeaturePresent(PF_XMMI64_INSTRUCTIONS_AVAILABLE))

Vulnerable code: 	void (*decode)(NSC_CONTEXT* context);
	void (*encode)(NSC_CONTEXT* context, const BYTE* BitmapData,
static void nsc_decode(NSC_CONTEXT* context)
	UINT16 rw = ROUND_UP_TO(context->width, 8);
	BYTE shift = context->ColorLossLevel - 1; /* colorloss recovery + YCoCg shift */
	BYTE* bmpdata = context->BitmapData;
static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize)
	*((UINT32*)out) = *((UINT32*)in);
static void nsc_rle_decompress_data(NSC_CONTEXT* context)
			nsc_rle_decode(rle, context->priv->PlaneBuffers[i], originalSize);
	PROFILER_ENTER(context->priv->prof_nsc_rle_decompress_data)
	nsc_rle_decompress_data(context);
	PROFILER_EXIT(context->priv->prof_nsc_rle_decompress_data)
	PROFILER_ENTER(context->priv->prof_nsc_decode)
	context->decode(context);
	PROFILER_EXIT(context->priv->prof_nsc_decode)
static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,
static void nsc_encode_subsampling(NSC_CONTEXT* context)
	BYTE* co_dst;
	BYTE* cg_dst;
	INT8* co_src0;
	INT8* co_src1;
	INT8* cg_src0;
	INT8* cg_src1;
		co_dst = context->priv->PlaneBuffers[1] + y * (tempWidth >> 1);
		cg_dst = context->priv->PlaneBuffers[2] + y * (tempWidth >> 1);
		co_src0 = (INT8*) context->priv->PlaneBuffers[1] + (y << 1) * tempWidth;
		co_src1 = co_src0 + tempWidth;
		cg_src0 = (INT8*) context->priv->PlaneBuffers[2] + (y << 1) * tempWidth;
		cg_src1 = cg_src0 + tempWidth;
void nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata, UINT32 rowstride)
	nsc_encode_argb_to_aycocg(context, bmpdata, rowstride);
		nsc_encode_subsampling(context);
static UINT32 nsc_rle_encode(BYTE* in, BYTE* out, UINT32 originalSize)
FREERDP_LOCAL void nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata,
static void nsc_encode_sse2(NSC_CONTEXT* context, const BYTE* data,
Fixed code: 	BOOL (*decode)(NSC_CONTEXT* context);
	BOOL (*encode)(NSC_CONTEXT* context, const BYTE* BitmapData,
static BOOL nsc_decode(NSC_CONTEXT* context)
	UINT16 rw;
	BYTE shift;
	BYTE* bmpdata;
	size_t pos = 0;

	if (!context)
		return FALSE;

	rw = ROUND_UP_TO(context->width, 8);
	shift = context->ColorLossLevel - 1; /* colorloss recovery + YCoCg shift */
	bmpdata = context->BitmapData;

	if (!bmpdata)
		return FALSE;

			if (pos + 4 > context->BitmapDataLength)
				return FALSE;

			pos += 4;

	return TRUE;
static BOOL nsc_rle_decode(BYTE* in, BYTE* out, UINT32 outSize, UINT32 originalSize)
			if (outSize < 1)
				return FALSE;

			outSize--;
			if (outSize < len)
				return FALSE;

			outSize -= len;
			if (outSize < 1)
				return FALSE;

			outSize--;
	if ((outSize < 4) || (left < 4))
		return FALSE;

	memcpy(out, in, 4);
	return TRUE;
static BOOL nsc_rle_decompress_data(NSC_CONTEXT* context)

	if (!context)
		return FALSE;

		{
			if (context->priv->PlaneBuffersLength < originalSize)
				return FALSE;

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

		}

	return TRUE;
	{
		BOOL rc;
		PROFILER_ENTER(context->priv->prof_nsc_rle_decompress_data)
		rc = nsc_rle_decompress_data(context);
		PROFILER_EXIT(context->priv->prof_nsc_rle_decompress_data)

		if (!rc)
			return FALSE;
	}
	{
		BOOL rc;
		PROFILER_ENTER(context->priv->prof_nsc_decode)
		rc = context->decode(context);
		PROFILER_EXIT(context->priv->prof_nsc_decode)

		if (!rc)
			return FALSE;
	}

static BOOL nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data,

	if (!context || data || (scanline == 0))
		return FALSE;

	if (context->priv->PlaneBuffersLength < rw * scanline)
		return FALSE;

	if (rw < scanline * 2)
		return FALSE;


	return TRUE;
static BOOL nsc_encode_subsampling(NSC_CONTEXT* context)

	if (!context)
		return FALSE;

	if (tempHeight == 0)
		return FALSE;

	if (tempWidth > context->priv->PlaneBuffersLength / tempHeight)
		return FALSE;

		BYTE* co_dst = context->priv->PlaneBuffers[1] + y * (tempWidth >> 1);
		BYTE* cg_dst = context->priv->PlaneBuffers[2] + y * (tempWidth >> 1);
		const INT8* co_src0 = (INT8*) context->priv->PlaneBuffers[1] + (y << 1) * tempWidth;
		const INT8* co_src1 = co_src0 + tempWidth;
		const INT8* cg_src0 = (INT8*) context->priv->PlaneBuffers[2] + (y << 1) * tempWidth;
		const INT8* cg_src1 = cg_src0 + tempWidth;

	return TRUE;
BOOL nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata, UINT32 rowstride)
	if (!context || !bmpdata || (rowstride == 0))
		return FALSE;

	if (!nsc_encode_argb_to_aycocg(context, bmpdata, rowstride))
		return FALSE;
		if (!nsc_encode_subsampling(context))
			return FALSE;

	return TRUE;
static UINT32 nsc_rle_encode(const BYTE* in, BYTE* out, UINT32 originalSize)
FREERDP_LOCAL BOOL nsc_encode(NSC_CONTEXT* context, const BYTE* bmpdata,
static BOOL nsc_encode_sse2(NSC_CONTEXT* context, const BYTE* data,

	return TRUE;