From 2648257caa992c0a96c8c664fec1ac8dc1060eef Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 12 Dec 2022 13:28:36 +0100 Subject: [PATCH] [codec,interleaved] add proper debug logging log reason for decoder to fail --- libfreerdp/codec/include/bitmap.c | 27 ++++++++--- libfreerdp/codec/interleaved.c | 78 ++++++++++++++++++++++++++----- 2 files changed, 87 insertions(+), 18 deletions(-) diff --git a/libfreerdp/codec/include/bitmap.c b/libfreerdp/codec/include/bitmap.c index f82e14b53..818c2725f 100644 --- a/libfreerdp/codec/include/bitmap.c +++ b/libfreerdp/codec/include/bitmap.c @@ -31,7 +31,10 @@ static INLINE BYTE* WRITEFGBGIMAGE(BYTE* pbDest, const BYTE* pbDestEnd, UINT32 r BYTE mask = 0x01; if (cBits > 8) + { + WLog_ERR(TAG, "[%s] cBits %d > 8", __FUNCTION__, cBits); return NULL; + } if (!ENSURE_CAPACITY(pbDest, pbDestEnd, cBits)) return NULL; @@ -62,7 +65,10 @@ static INLINE BYTE* WRITEFIRSTLINEFGBGIMAGE(BYTE* pbDest, const BYTE* pbDestEnd, BYTE mask = 0x01; if (cBits > 8) + { + WLog_ERR(TAG, "[%s] cBits %d > 8", __FUNCTION__, cBits); return NULL; + } if (!ENSURE_CAPACITY(pbDest, pbDestEnd, cBits)) return NULL; @@ -104,10 +110,18 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY RLEEXTRA if ((rowDelta == 0) || (rowDelta < width)) + { + WLog_ERR(TAG, "[%s] Invalid arguments: rowDelta=%" PRIu32 " == 0 || < width=%" PRIu32, + __FUNCTION__, rowDelta, width); return FALSE; + } if (!pbSrcBuffer || !pbDestBuffer) + { + WLog_ERR(TAG, "[%s] Invalid arguments: pbSrcBuffer=%p, pbDestBuffer=%p", __FUNCTION__, + pbSrcBuffer, pbDestBuffer); return FALSE; + } pbEnd = pbSrcBuffer + cbSrcBuffer; pbDestEnd = pbDestBuffer + rowDelta * height; @@ -201,7 +215,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY if (code == LITE_SET_FG_FG_RUN || code == MEGA_MEGA_SET_FG_RUN) { - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; SRCREADPIXEL(fgPel, pbSrc); SRCNEXTPIXEL(pbSrc); @@ -233,11 +247,11 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY case MEGA_MEGA_DITHERED_RUN: runLength = ExtractRunLength(code, pbSrc, pbEnd, &advance); pbSrc = pbSrc + advance; - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; SRCREADPIXEL(pixelA, pbSrc); SRCNEXTPIXEL(pbSrc); - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; SRCREADPIXEL(pixelB, pbSrc); SRCNEXTPIXEL(pbSrc); @@ -258,7 +272,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY case MEGA_MEGA_COLOR_RUN: runLength = ExtractRunLength(code, pbSrc, pbEnd, &advance); pbSrc = pbSrc + advance; - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; SRCREADPIXEL(pixelA, pbSrc); SRCNEXTPIXEL(pbSrc); @@ -280,7 +294,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY runLength = ExtractRunLength(code, pbSrc, pbEnd, &advance); pbSrc = pbSrc + advance; - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; if (code == LITE_SET_FG_FGBG_IMAGE || code == MEGA_MEGA_SET_FGBG_IMAGE) { @@ -348,7 +362,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY return FALSE; UNROLL(runLength, { - if (pbSrc >= pbEnd) + if (!buffer_within_range(pbSrc, pbEnd)) return FALSE; SRCREADPIXEL(temp, pbSrc); SRCNEXTPIXEL(pbSrc); @@ -420,6 +434,7 @@ static INLINE BOOL RLEDECOMPRESS(const BYTE* pbSrcBuffer, UINT32 cbSrcBuffer, BY break; default: + WLog_ERR(TAG, "[%s] invalid code 0x%08" PRIx32, __FUNCTION__, code); return FALSE; } } diff --git a/libfreerdp/codec/interleaved.c b/libfreerdp/codec/interleaved.c index 85c7363ad..73000ad1c 100644 --- a/libfreerdp/codec/interleaved.c +++ b/libfreerdp/codec/interleaved.c @@ -21,6 +21,7 @@ * limitations under the License. */ +#include #include #include @@ -95,6 +96,20 @@ static const BYTE g_MaskSpecialFgBg2 = 0x05; static const BYTE g_MaskRegularRunLength = 0x1F; static const BYTE g_MaskLiteRunLength = 0x0F; +#define buffer_within_range(pbSrc, pbEnd) \ + buffer_within_range_((pbSrc), (pbEnd), __FUNCTION__, __FILE__, __LINE__) +static INLINE BOOL buffer_within_range_(const void* pbSrc, const void* pbEnd, const char* fkt, + const char* file, size_t line) +{ + WINPR_UNUSED(file); + if (pbSrc >= pbEnd) + { + WLog_ERR(TAG, "[%s:%" PRIuz "] pbSrc=%p >= pbEnd=%p", fkt, line, pbSrc, pbEnd); + return FALSE; + } + return TRUE; +} + /** * Reads the supplied order header and extracts the compression * order code ID. @@ -131,7 +146,11 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const UINT32 runLength = 0; UINT32 ladvance = 1; - if (pbOrderHdr >= pbEnd) + WINPR_ASSERT(pbOrderHdr); + WINPR_ASSERT(pbEnd); + WINPR_ASSERT(advance); + + if (!buffer_within_range(pbOrderHdr, pbEnd)) return 0; switch (code) @@ -141,7 +160,7 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const if (runLength == 0) { - if (pbOrderHdr + 1 >= pbEnd) + if (!buffer_within_range(pbOrderHdr + 1, pbEnd)) return 0; runLength = (*(pbOrderHdr + 1)) + 1; ladvance += 1; @@ -158,7 +177,7 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const if (runLength == 0) { - if (pbOrderHdr + 1 >= pbEnd) + if (!buffer_within_range(pbOrderHdr + 1, pbEnd)) return 0; runLength = (*(pbOrderHdr + 1)) + 1; @@ -180,7 +199,7 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const if (runLength == 0) { /* An extended (MEGA) run. */ - if (pbOrderHdr + 1 >= pbEnd) + if (!buffer_within_range(pbOrderHdr + 1, pbEnd)) return 0; runLength = (*(pbOrderHdr + 1)) + 32; @@ -196,7 +215,7 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const if (runLength == 0) { /* An extended (MEGA) run. */ - if (pbOrderHdr + 1 >= pbEnd) + if (!buffer_within_range(pbOrderHdr + 1, pbEnd)) return 0; runLength = (*(pbOrderHdr + 1)) + 16; @@ -213,7 +232,7 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const case MEGA_MEGA_FGBG_IMAGE: case MEGA_MEGA_SET_FGBG_IMAGE: case MEGA_MEGA_COLOR_IMAGE: - if (pbOrderHdr + 2 >= pbEnd) + if (!buffer_within_range(pbOrderHdr + 2, pbEnd)) return 0; runLength = ((UINT16)pbOrderHdr[1]) | ((UINT16)(pbOrderHdr[2] << 8)); @@ -225,20 +244,32 @@ static INLINE UINT32 ExtractRunLength(UINT32 code, const BYTE* pbOrderHdr, const return runLength; } -static INLINE BOOL ensure_capacity(const BYTE* start, const BYTE* end, size_t size, size_t base) +#define ensure_capacity(start, end, size, base) \ + ensure_capacity_((start), (end), (size), (base), __FUNCTION__, __FILE__, __LINE__) +static INLINE BOOL ensure_capacity_(const BYTE* start, const BYTE* end, size_t size, size_t base, + const char* fkt, const char* file, size_t line) { const size_t available = (uintptr_t)end - (uintptr_t)start; const BOOL rc = available >= size * base; - return rc && (start <= end); + const BOOL res = rc && (start <= end); + + if (!res) + WLog_ERR(TAG, + "[%s:%" PRIuz "] failed: start=%p <= end=%p, available=%" PRIuz " >= size=%" PRIuz + " * base=%" PRIuz, + fkt, line, start, end, available, size, base); + return res; } static INLINE void write_pixel_8(BYTE* _buf, BYTE _pix) { + WINPR_ASSERT(_buf); *_buf = _pix; } static INLINE void write_pixel_24(BYTE* _buf, UINT32 _pix) { + WINPR_ASSERT(_buf); (_buf)[0] = (BYTE)(_pix); (_buf)[1] = (BYTE)((_pix) >> 8); (_buf)[2] = (BYTE)((_pix) >> 16); @@ -246,6 +277,7 @@ static INLINE void write_pixel_24(BYTE* _buf, UINT32 _pix) static INLINE void write_pixel_16(BYTE* _buf, UINT16 _pix) { + WINPR_ASSERT(_buf); _buf[0] = _pix & 0xFF; _buf[1] = (_pix >> 8) & 0xFF; } @@ -343,7 +375,11 @@ BOOL interleaved_decompress(BITMAP_INTERLEAVED_CONTEXT* interleaved, const BYTE* UINT32 BufferSize; if (!interleaved || !pSrcData || !pDstData) + { + WLog_ERR(TAG, "[%s] invalid arguments: interleaved=%p, pSrcData=%p, pDstData=%p", + __FUNCTION__, interleaved, pSrcData, pDstData); return FALSE; + } switch (bpp) { @@ -368,7 +404,7 @@ BOOL interleaved_decompress(BITMAP_INTERLEAVED_CONTEXT* interleaved, const BYTE* break; default: - WLog_ERR(TAG, "Invalid color depth %" PRIu32 "", bpp); + WLog_ERR(TAG, "[%s] Invalid color depth %" PRIu32 "", __FUNCTION__, bpp); return FALSE; } @@ -381,14 +417,20 @@ BOOL interleaved_decompress(BITMAP_INTERLEAVED_CONTEXT* interleaved, const BYTE* } if (!interleaved->TempBuffer) + { + WLog_ERR(TAG, "[%s] interleaved->TempBuffer=%p", __FUNCTION__, interleaved->TempBuffer); return FALSE; + } switch (bpp) { case 24: if (!RleDecompress24to24(pSrcData, SrcSize, interleaved->TempBuffer, scanline, nSrcWidth, nSrcHeight)) + { + WLog_ERR(TAG, "[%s] RleDecompress24to24 failed", __FUNCTION__); return FALSE; + } break; @@ -396,24 +438,36 @@ BOOL interleaved_decompress(BITMAP_INTERLEAVED_CONTEXT* interleaved, const BYTE* case 15: if (!RleDecompress16to16(pSrcData, SrcSize, interleaved->TempBuffer, scanline, nSrcWidth, nSrcHeight)) + { + WLog_ERR(TAG, "[%s] RleDecompress16to16 failed", __FUNCTION__); return FALSE; + } break; case 8: if (!RleDecompress8to8(pSrcData, SrcSize, interleaved->TempBuffer, scanline, nSrcWidth, nSrcHeight)) + { + WLog_ERR(TAG, "[%s] RleDecompress8to8 failed", __FUNCTION__); return FALSE; + } break; default: + WLog_ERR(TAG, "[%s] Invalid color depth %" PRIu32 "", __FUNCTION__, bpp); return FALSE; } - return freerdp_image_copy(pDstData, DstFormat, nDstStep, nXDst, nYDst, nDstWidth, nDstHeight, - interleaved->TempBuffer, SrcFormat, scanline, 0, 0, palette, - FREERDP_FLIP_VERTICAL); + if (!freerdp_image_copy(pDstData, DstFormat, nDstStep, nXDst, nYDst, nDstWidth, nDstHeight, + interleaved->TempBuffer, SrcFormat, scanline, 0, 0, palette, + FREERDP_FLIP_VERTICAL)) + { + WLog_ERR(TAG, "[%s] freerdp_image_copy failed", __FUNCTION__); + return FALSE; + } + return TRUE; } BOOL interleaved_compress(BITMAP_INTERLEAVED_CONTEXT* interleaved, BYTE* pDstData, UINT32* pDstSize,