m d1112c279bd1a327e8e4d0b5f371458bf2579659 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 22 Oct 2018 16:52:21 +0200 Subject: [PATCH] Fixed CVE-2018-8788 Thanks to Eyal Itkin from Check Point Software Technologies. --- include/freerdp/codec/nsc.h | 4 +- libfreerdp/codec/nsc.c | 94 +++++++++++++++++++++++++++++------ libfreerdp/codec/nsc_encode.c | 62 ++++++++++++++++------- libfreerdp/codec/nsc_encode.h | 2 +- libfreerdp/codec/nsc_sse2.c | 4 +- 5 files changed, 130 insertions(+), 36 deletions(-) diff --git a/include/freerdp/codec/nsc.h b/include/freerdp/codec/nsc.h index 21e575ad27..cb16570db8 100644 --- a/include/freerdp/codec/nsc.h +++ b/include/freerdp/codec/nsc.h @@ -77,8 +77,8 @@ struct _NSC_CONTEXT /* 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 d0547cd333..bad0aec1c9 100644 --- a/libfreerdp/codec/nsc.c +++ b/libfreerdp/codec/nsc.c @@ -42,13 +42,24 @@ #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++) { @@ -80,6 +91,11 @@ static void nsc_decode(NSC_CONTEXT* context) 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); @@ -90,9 +106,11 @@ static void nsc_decode(NSC_CONTEXT* context) 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; @@ -105,6 +123,10 @@ static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize) if (left == 5) { + if (outSize < 1) + return FALSE; + + outSize--; *out++ = value; left--; } @@ -124,26 +146,42 @@ static void nsc_rle_decode(BYTE* in, BYTE* out, UINT32 originalSize) 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++) @@ -152,14 +190,30 @@ static void nsc_rle_decompress_data(NSC_CONTEXT* context) 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) @@ -396,13 +450,25 @@ BOOL nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, 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, diff --git a/libfreerdp/codec/nsc_encode.c b/libfreerdp/codec/nsc_encode.c index 492f170dc8..d2456fb939 100644 --- a/libfreerdp/codec/nsc_encode.c +++ b/libfreerdp/codec/nsc_encode.c @@ -51,6 +51,7 @@ static BOOL nsc_context_initialize_encode(NSC_CONTEXT* context) for (i = 0; i < 5; i++) { BYTE* tmp = (BYTE*) realloc(context->priv->PlaneBuffers[i], length); + if (!tmp) goto fail; @@ -87,7 +88,7 @@ static BOOL nsc_context_initialize_encode(NSC_CONTEXT* context) 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; @@ -104,10 +105,20 @@ static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data, 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; @@ -242,31 +253,37 @@ static void nsc_encode_argb_to_aycocg(NSC_CONTEXT* context, const BYTE* data, 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++) { @@ -280,19 +297,28 @@ static void nsc_encode_subsampling(NSC_CONTEXT* context) 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; diff --git a/libfreerdp/codec/nsc_encode.h b/libfreerdp/codec/nsc_encode.h index e220de4072..784ccb6e35 100644 --- a/libfreerdp/codec/nsc_encode.h +++ b/libfreerdp/codec/nsc_encode.h @@ -24,7 +24,7 @@ #include -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 149e80a83c..966525f16d 100644 --- a/libfreerdp/codec/nsc_sse2.c +++ b/libfreerdp/codec/nsc_sse2.c @@ -385,7 +385,7 @@ static void nsc_encode_subsampling_sse2(NSC_CONTEXT* context) } } -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); @@ -394,6 +394,8 @@ static void nsc_encode_sse2(NSC_CONTEXT* context, const BYTE* data, { nsc_encode_subsampling_sse2(context); } + + return TRUE; } void nsc_init_sse2(NSC_CONTEXT* context)