From c43faeec0afad1621da81c2fe762d240b30ff269 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 22 Jan 2015 13:06:37 +0100 Subject: [PATCH 1/5] Removed broken buffer size check. To check the decoded h264 frame size against the output buffer is wrong. The size of the output buffer must only hold the data defined by the region rectangles. --- libfreerdp/codec/h264.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libfreerdp/codec/h264.c b/libfreerdp/codec/h264.c index f42f1be0f..538591fcf 100644 --- a/libfreerdp/codec/h264.c +++ b/libfreerdp/codec/h264.c @@ -435,7 +435,6 @@ int h264_decompress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, int width, height; BYTE* pYUVPoint[3]; RDPGFX_RECT16* rect; - int UncompressedSize; primitives_t *prims = primitives_get(); if (!h264) @@ -452,11 +451,6 @@ int h264_decompress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, if ((status = h264->subsystem->Decompress(h264, pSrcData, SrcSize)) < 0) return status; - UncompressedSize = h264->width * h264->height * 4; - - if (UncompressedSize > (nDstStep * nDstHeight)) - return -1; - pYUVData = h264->pYUVData; iStride = h264->iStride; From d42261f5eb4cd70bfad31e586a032b59f6977685 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 22 Jan 2015 13:22:53 +0100 Subject: [PATCH 2/5] Added destination buffer width to h264_decompress. Added proper region limit checks in h264_decompress. --- libfreerdp/codec/h264.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libfreerdp/codec/h264.c b/libfreerdp/codec/h264.c index 538591fcf..d3fdf2a0a 100644 --- a/libfreerdp/codec/h264.c +++ b/libfreerdp/codec/h264.c @@ -423,7 +423,8 @@ static H264_CONTEXT_SUBSYSTEM g_Subsystem_libavcodec = #endif int h264_decompress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, - BYTE** ppDstData, DWORD DstFormat, int nDstStep, int nDstHeight, RDPGFX_RECT16* regionRects, int numRegionRects) + BYTE** ppDstData, DWORD DstFormat, int nDstStep, int nDstWidth, + int nDstHeight, RDPGFX_RECT16* regionRects, int numRegionRects) { int index; int status; @@ -458,6 +459,18 @@ int h264_decompress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, { rect = &(regionRects[index]); + /* Check, if the ouput rectangle is valid in decoded h264 frame. */ + if ((rect->right > h264->width) || (rect->left > h264->width)) + return -1; + if ((rect->top > h264->height) || (rect->bottom > h264->height)) + return -1; + + /* Check, if the output rectangle is valid in destination buffer. */ + if ((rect->right > nDstWidth) || (rect->left > nDstWidth)) + return -1; + if ((rect->bottom > nDstHeight) || (rect->top > nDstHeight)) + return -1; + width = rect->right - rect->left; height = rect->bottom - rect->top; From 3c7b611041255c4f61f3e2b7026026f2b5f96d22 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 22 Jan 2015 13:23:59 +0100 Subject: [PATCH 3/5] Added destination buffer width to h264_decompress. --- include/freerdp/codec/h264.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/freerdp/codec/h264.h b/include/freerdp/codec/h264.h index 8e835a9e7..2c9900805 100644 --- a/include/freerdp/codec/h264.h +++ b/include/freerdp/codec/h264.h @@ -61,7 +61,8 @@ extern "C" { FREERDP_API int h264_compress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, BYTE** ppDstData, UINT32* pDstSize); FREERDP_API int h264_decompress(H264_CONTEXT* h264, BYTE* pSrcData, UINT32 SrcSize, - BYTE** ppDstData, DWORD DstFormat, int nDstStep, int nDstHeight, RDPGFX_RECT16* regionRects, int numRegionRect); + BYTE** ppDstData, DWORD DstFormat, int nDstStep, int nDstWidth, int nDstHeight, + RDPGFX_RECT16* regionRects, int numRegionRect); FREERDP_API int h264_context_reset(H264_CONTEXT* h264); From 7fc9f98d3cfd6f09f3a87f44f131ef67b727907b Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 22 Jan 2015 13:24:15 +0100 Subject: [PATCH 4/5] Updated h264_decompress arguments. --- libfreerdp/gdi/gfx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c index 9447fa01f..1b7076f25 100644 --- a/libfreerdp/gdi/gfx.c +++ b/libfreerdp/gdi/gfx.c @@ -356,7 +356,8 @@ int gdi_SurfaceCommand_H264(rdpGdi* gdi, RdpgfxClientContext* context, RDPGFX_SU DstData = surface->data; status = h264_decompress(gdi->codecs->h264, bs->data, bs->length, &DstData, - PIXEL_FORMAT_XRGB32, surface->scanline , surface->height, meta->regionRects, meta->numRegionRects); + PIXEL_FORMAT_XRGB32, surface->scanline , surface->width, surface->height, + meta->regionRects, meta->numRegionRects); if (status < 0) { From dd9e1879b6c2012ac4c72ce4bea201b1fdce37d9 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 22 Jan 2015 13:24:33 +0100 Subject: [PATCH 5/5] Updated h264_decompress arguments. --- client/X11/xf_gfx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/X11/xf_gfx.c b/client/X11/xf_gfx.c index 526181def..673d046c1 100644 --- a/client/X11/xf_gfx.c +++ b/client/X11/xf_gfx.c @@ -353,7 +353,8 @@ int xf_SurfaceCommand_H264(xfContext* xfc, RdpgfxClientContext* context, RDPGFX_ DstData = surface->data; status = h264_decompress(xfc->codecs->h264, bs->data, bs->length, &DstData, - surface->format, surface->scanline , surface->height, meta->regionRects, meta->numRegionRects); + surface->format, surface->scanline , surface->width, + surface->height, meta->regionRects, meta->numRegionRects); if (status < 0) {