From 0cbc46dc34d994204017bf419c94e9c5fdd44f4d Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 5 Aug 2015 17:19:46 +0200 Subject: [PATCH 1/7] Fixed pointer update decoding with NULL mask. --- libfreerdp/codec/color.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index e7cc8b334..40bd5162a 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -1481,12 +1481,14 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n if (!vFlip) { xorBits = &xorMask[xorStep * y]; - andBits = &andMask[andStep * y]; + if (andMask) + andBits = &andMask[andStep * y]; } else { xorBits = &xorMask[xorStep * (nHeight - y - 1)]; - andBits = &andMask[andStep * (nHeight - y - 1)]; + if (andMask) + andBits = &andMask[andStep * (nHeight - y - 1)]; } for (x = 0; x < nWidth; x++) @@ -1531,12 +1533,14 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n if (!vFlip) { - andBits = &andMask[andStep * y]; + if (andMask) + andBits = &andMask[andStep * y]; xorBits = &xorMask[xorStep * y]; } else { - andBits = &andMask[andStep * (nHeight - y - 1)]; + if (andMask) + andBits = &andMask[andStep * (nHeight - y - 1)]; xorBits = &xorMask[xorStep * (nHeight - y - 1)]; } @@ -1563,8 +1567,12 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n xorBits += xorBytesPerPixel; - andPixel = (*andBits & andBit) ? 1 : 0; - if (!(andBit >>= 1)) { andBits++; andBit = 0x80; } + andPixel = 0; + if (andMask) + { + andPixel = (*andBits & andBit) ? 1 : 0; + if (!(andBit >>= 1)) { andBits++; andBit = 0x80; } + } if (andPixel) { From 4df5e2b9983dc0cf03eaf92c4904f1bcfdef7012 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 5 Aug 2015 17:32:38 +0200 Subject: [PATCH 2/7] Fixed argument checks. --- libfreerdp/codec/color.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index 40bd5162a..1a22f6e7a 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -1463,12 +1463,18 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n andStep = (nWidth + 7) / 8; andStep += (andStep % 2); + if (!xorMask) + return -1; + if (dstBytesPerPixel == 4) { UINT32* pDstPixel; if (xorBpp == 1) { + if (!andMask) + return -1; + xorStep = (nWidth + 7) / 8; xorStep += (xorStep % 2); @@ -1481,14 +1487,12 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n if (!vFlip) { xorBits = &xorMask[xorStep * y]; - if (andMask) - andBits = &andMask[andStep * y]; + andBits = &andMask[andStep * y]; } else { xorBits = &xorMask[xorStep * (nHeight - y - 1)]; - if (andMask) - andBits = &andMask[andStep * (nHeight - y - 1)]; + andBits = &andMask[andStep * (nHeight - y - 1)]; } for (x = 0; x < nWidth; x++) From 416ac0baea44388a7f6532fae6206894da9e57bc Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Wed, 5 Aug 2015 17:32:46 +0200 Subject: [PATCH 3/7] Fixed return and argument checks. --- client/X11/xf_graphics.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/client/X11/xf_graphics.c b/client/X11/xf_graphics.c index 4ea4bb051..060ab0dcf 100644 --- a/client/X11/xf_graphics.c +++ b/client/X11/xf_graphics.c @@ -237,11 +237,13 @@ BOOL xf_Pointer_New(rdpContext* context, rdpPointer* pointer) return FALSE; } - if ((pointer->andMaskData != 0) && (pointer->xorMaskData != 0)) - { - freerdp_image_copy_from_pointer_data((BYTE*) ci.pixels, PIXEL_FORMAT_ARGB32, + if (freerdp_image_copy_from_pointer_data((BYTE*) ci.pixels, PIXEL_FORMAT_ARGB32, pointer->width * 4, 0, 0, pointer->width, pointer->height, - pointer->xorMaskData, pointer->andMaskData, pointer->xorBpp, xfc->palette); + pointer->xorMaskData, pointer->andMaskData, pointer->xorBpp, xfc->palette) < 0) + { + free(ci.pixels); + xf_unlock_x11(xfc, FALSE); + return FALSE; } ((xfPointer*) pointer)->cursor = XcursorImageLoadCursor(xfc->display, &ci); From 4a62e6bee435eb2cde877b49412e762e7c7fc4e9 Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 6 Aug 2015 11:24:42 +0200 Subject: [PATCH 4/7] Added length arguments and checks. --- .../Android/FreeRDPCore/jni/android_freerdp.c | 2 +- client/X11/xf_graphics.c | 9 ++- include/freerdp/codec/color.h | 13 ++-- libfreerdp/codec/color.c | 65 ++++++++++++------- 4 files changed, 57 insertions(+), 32 deletions(-) diff --git a/client/Android/FreeRDPCore/jni/android_freerdp.c b/client/Android/FreeRDPCore/jni/android_freerdp.c index ea6a0030b..4fc47b581 100644 --- a/client/Android/FreeRDPCore/jni/android_freerdp.c +++ b/client/Android/FreeRDPCore/jni/android_freerdp.c @@ -1148,7 +1148,7 @@ static void copy_pixel_buffer(UINT8* dstBuf, UINT8* srcBuf, int x, int y, int wi int length; int scanline; UINT8 *dstp, *srcp; - + length = width * bpp; scanline = wBuf * bpp; diff --git a/client/X11/xf_graphics.c b/client/X11/xf_graphics.c index 060ab0dcf..6adbab3ae 100644 --- a/client/X11/xf_graphics.c +++ b/client/X11/xf_graphics.c @@ -237,9 +237,12 @@ BOOL xf_Pointer_New(rdpContext* context, rdpPointer* pointer) return FALSE; } - if (freerdp_image_copy_from_pointer_data((BYTE*) ci.pixels, PIXEL_FORMAT_ARGB32, - pointer->width * 4, 0, 0, pointer->width, pointer->height, - pointer->xorMaskData, pointer->andMaskData, pointer->xorBpp, xfc->palette) < 0) + if (freerdp_image_copy_from_pointer_data( + (BYTE*) ci.pixels, PIXEL_FORMAT_ARGB32, + pointer->width * 4, 0, 0, pointer->width, pointer->height, + pointer->xorMaskData, pointer->lengthXorMask, + pointer->andMaskData, pointer->lengthAndMask, + pointer->xorBpp, xfc->palette) < 0) { free(ci.pixels); xf_unlock_x11(xfc, FALSE); diff --git a/include/freerdp/codec/color.h b/include/freerdp/codec/color.h index 9f8aca695..9b3457b65 100644 --- a/include/freerdp/codec/color.h +++ b/include/freerdp/codec/color.h @@ -376,12 +376,12 @@ extern "C" { #define BGR16_RGB32(_r, _g, _b, _p) \ GetBGR16(_r, _g, _b, _p); \ - RGB_565_888(_r, _g, _b); \ + RGB_565_888(_r, _g, _b); \ _p = RGB32(_r, _g, _b); #define RGB32_RGB16(_r, _g, _b, _p) \ GetRGB32(_r, _g, _b, _p); \ - RGB_888_565(_r, _g, _b); \ + RGB_888_565(_r, _g, _b); \ _p = RGB565(_r, _g, _b); #define RGB15_RGB16(_r, _g, _b, _p) \ @@ -452,8 +452,11 @@ FREERDP_API void freerdp_clrconv_free(HCLRCONV clrconv); FREERDP_API int freerdp_image_copy_from_monochrome(BYTE* pDstData, UINT32 DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, BYTE* pSrcData, UINT32 backColor, UINT32 foreColor, BYTE* palette); -FREERDP_API int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int nDstStep, int nXDst, int nYDst, - int nWidth, int nHeight, BYTE* xorMask, BYTE* andMask, UINT32 xorBpp, BYTE* palette); +FREERDP_API int int freerdp_image_copy_from_pointer_data( + BYTE* pDstData, UINT32 DstFormat, int nDstStep, + int nXDst, int nYDst, int nWidth, int nHeight, BYTE* xorMask, + UINT32 xorMaskLength, BYTE* andMask, UINT32 andMaskLength, + UINT32 xorBpp, BYTE* palette); FREERDP_API int freerdp_image8_copy(BYTE* pDstData, DWORD DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, BYTE* pSrcData, DWORD SrcFormat, int nSrcStep, int nXSrc, int nYSrc, BYTE* palette); @@ -472,7 +475,7 @@ FREERDP_API int freerdp_image_move(BYTE* pData, DWORD Format, int nStep, int nXD int nWidth, int nHeight, int nXSrc, int nYSrc); FREERDP_API int freerdp_image_fill(BYTE* pDstData, DWORD DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, UINT32 color); - + FREERDP_API int freerdp_image_copy_from_retina(BYTE* pDstData, DWORD DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, BYTE* pSrcData, int nSrcStep, int nXSrc, int nYSrc); diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index 1a22f6e7a..600214030 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -1036,23 +1036,23 @@ BYTE* freerdp_icon_convert(BYTE* srcData, BYTE* dstData, BYTE* mask, int width, /* Server sends 16 bpp field, but data is usually 15-bit 555 */ bpp = 15; } - + data = freerdp_image_flip(srcData, dstData, width, height, bpp); dstData = freerdp_image_convert(data, NULL, width, height, bpp, 32, clrconv); _aligned_free(data); - /* Read the AND alpha plane */ + /* Read the AND alpha plane */ if (bpp < 32) { maskIndex = 0; icon = (UINT32*) dstData; - + for (y = 0; y < height; y++) { for (x = 0; x < width-7; x+=8) { bmask = mask[maskIndex++]; - + for (bit = 0; bit < 8; bit++) if ((bmask & (0x80 >> bit)) == 0) { @@ -1061,11 +1061,11 @@ BYTE* freerdp_icon_convert(BYTE* srcData, BYTE* dstData, BYTE* mask, int width, *tmp |= 0xFF000000; } } - + if ((width % 8) != 0) { bmask = mask[maskIndex++]; - + for (bit = 0; bit < width % 8; bit++) if ((bmask & (0x80 >> bit)) == 0) { @@ -1074,7 +1074,7 @@ BYTE* freerdp_icon_convert(BYTE* srcData, BYTE* dstData, BYTE* mask, int width, *tmp |= 0xFF000000; } } - + /* Skip padding */ if ((width % 32) != 0) maskIndex += (32 - (width % 32)) / 8; @@ -1426,8 +1426,12 @@ void freerdp_alpha_cursor_convert(BYTE* alphaData, BYTE* xorMask, BYTE* andMask, * http://msdn.microsoft.com/en-us/library/windows/hardware/ff556138/ */ -int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int nDstStep, int nXDst, int nYDst, - int nWidth, int nHeight, BYTE* xorMask, BYTE* andMask, UINT32 xorBpp, BYTE* palette) +int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, + int nDstStep, int nXDst, int nYDst, + int nWidth, int nHeight, BYTE* xorMask, + UINT32 xorMaskLength, BYTE* andMask, + UINT32 andMaskLength, UINT32 xorBpp, + BYTE* palette) { int x, y; BOOL vFlip; @@ -1463,7 +1467,7 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n andStep = (nWidth + 7) / 8; andStep += (andStep % 2); - if (!xorMask) + if (!xorMask || (xorMaskLength == 0)) return -1; if (dstBytesPerPixel == 4) @@ -1472,12 +1476,18 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n if (xorBpp == 1) { - if (!andMask) + if (!andMask || (andMaskLength == 0)) return -1; xorStep = (nWidth + 7) / 8; xorStep += (xorStep % 2); + if (xorStep * nHeight > xorMaskLength) + return -1; + + if (andStep * nHeight > andMaskLength) + return -1; + pDstPixel = (UINT32*) &pDstData[(nYDst * nDstStep) + (nXDst * 4)]; for (y = 0; y < nHeight; y++) @@ -1531,6 +1541,15 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, int n return -1; } + if (xorStep * nHeight > xorMaskLength) + return -1; + + if (andMask) + { + if (andStep * nHeight > andMaskLength) + return -1; + } + for (y = 0; y < nHeight; y++) { andBit = 0x80; @@ -3682,52 +3701,52 @@ int freerdp_image_copy_from_retina(BYTE* pDstData, DWORD DstFormat, int nDstStep int srcBytesPerPixel; int dstBitsPerPixel; int dstBytesPerPixel; - + srcBitsPerPixel = 24; srcBytesPerPixel = 8; - + if (nSrcStep < 0) nSrcStep = srcBytesPerPixel * nWidth; - + dstBitsPerPixel = FREERDP_PIXEL_FORMAT_DEPTH(DstFormat); dstBytesPerPixel = (FREERDP_PIXEL_FORMAT_BPP(DstFormat) / 8); - + if (nDstStep < 0) nDstStep = dstBytesPerPixel * nWidth; - + nSrcPad = (nSrcStep - (nWidth * srcBytesPerPixel)); nDstPad = (nDstStep - (nWidth * dstBytesPerPixel)); - + if (dstBytesPerPixel == 4) { UINT32 R, G, B; BYTE* pSrcPixel; BYTE* pDstPixel; - + pSrcPixel = &pSrcData[(nYSrc * nSrcStep) + (nXSrc * 4)]; pDstPixel = &pDstData[(nYDst * nDstStep) + (nXDst * 4)]; - + for (y = 0; y < nHeight; y++) { for (x = 0; x < nWidth; x++) { /* simple box filter scaling, could be improved with better algorithm */ - + B = pSrcPixel[0] + pSrcPixel[4] + pSrcPixel[nSrcStep + 0] + pSrcPixel[nSrcStep + 4]; G = pSrcPixel[1] + pSrcPixel[5] + pSrcPixel[nSrcStep + 1] + pSrcPixel[nSrcStep + 5]; R = pSrcPixel[2] + pSrcPixel[6] + pSrcPixel[nSrcStep + 2] + pSrcPixel[nSrcStep + 6]; pSrcPixel += 8; - + *pDstPixel++ = (BYTE) (B >> 2); *pDstPixel++ = (BYTE) (G >> 2); *pDstPixel++ = (BYTE) (R >> 2); *pDstPixel++ = 0xFF; } - + pSrcPixel = &pSrcPixel[nSrcPad + nSrcStep]; pDstPixel = &pDstPixel[nDstPad]; } } - + return 1; } From be673f5766ca45c0f78db3a84575c079ee2efacf Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 6 Aug 2015 14:11:36 +0200 Subject: [PATCH 5/7] Fixed mac API. --- client/Mac/MRDPView.m | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/client/Mac/MRDPView.m b/client/Mac/MRDPView.m index f62e988c9..fb8f120ed 100644 --- a/client/Mac/MRDPView.m +++ b/client/Mac/MRDPView.m @@ -1074,11 +1074,19 @@ BOOL mf_Pointer_New(rdpContext* context, rdpPointer* pointer) if (!cursor_data) return FALSE; mrdpCursor->cursor_data = cursor_data; - - freerdp_image_copy_from_pointer_data(cursor_data, PIXEL_FORMAT_ARGB32, - pointer->width * 4, 0, 0, pointer->width, pointer->height, - pointer->xorMaskData, pointer->andMaskData, pointer->xorBpp, NULL); - + + if (freerdp_image_copy_from_pointer_data( + cursor_data, PIXEL_FORMAT_ARGB32, + pointer->width * 4, 0, 0, pointer->width, pointer->height, + pointer->xorMaskData, pointer->lengthXorMask, + pointer->andMaskData, pointer->lengthAndMask, + pointer->xorBpp, NULL) < 0) + { + free(cursor_data); + mrdpCursor->cursor_data = NULL; + return FALSE; + } + /* store cursor bitmap image in representation - required by NSImage */ bmiRep = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes:(unsigned char **) &cursor_data pixelsWide:rect.size.width From 7983c2f4571a505c0d54cb4dabcbca15ee989eeb Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Thu, 6 Aug 2015 14:43:39 +0200 Subject: [PATCH 6/7] Fixed duplicate int. --- include/freerdp/codec/color.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/freerdp/codec/color.h b/include/freerdp/codec/color.h index 9b3457b65..b682591e0 100644 --- a/include/freerdp/codec/color.h +++ b/include/freerdp/codec/color.h @@ -452,7 +452,7 @@ FREERDP_API void freerdp_clrconv_free(HCLRCONV clrconv); FREERDP_API int freerdp_image_copy_from_monochrome(BYTE* pDstData, UINT32 DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, BYTE* pSrcData, UINT32 backColor, UINT32 foreColor, BYTE* palette); -FREERDP_API int int freerdp_image_copy_from_pointer_data( +FREERDP_API int freerdp_image_copy_from_pointer_data( BYTE* pDstData, UINT32 DstFormat, int nDstStep, int nXDst, int nYDst, int nWidth, int nHeight, BYTE* xorMask, UINT32 xorMaskLength, BYTE* andMask, UINT32 andMaskLength, From 89227b97f03c240607b122029bdfaf4e373c906f Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 1 Sep 2015 09:25:41 +0200 Subject: [PATCH 7/7] Ignore AND mask for 32 bit pointer. --- libfreerdp/codec/color.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index 600214030..97a0d0c5b 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -1569,9 +1569,13 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, for (x = 0; x < nWidth; x++) { + BOOL ignoreAndMask = FALSE; + if (xorBpp == 32) { xorPixel = *((UINT32*) xorBits); + if (xorPixel & 0xFF000000) + ignoreAndMask = TRUE; } else if (xorBpp == 16) { @@ -1597,11 +1601,14 @@ int freerdp_image_copy_from_pointer_data(BYTE* pDstData, UINT32 DstFormat, if (!(andBit >>= 1)) { andBits++; andBit = 0x80; } } - if (andPixel) + /* Ignore the AND mask, if the color format already supplies alpha data. */ + if (andPixel && !ignoreAndMask) { - if (xorPixel == 0xFF000000) /* black */ + const UINT32 xorPixelMasked = xorPixel | 0xFF000000; + + if (xorPixelMasked == 0xFF000000) /* black */ *pDstPixel++ = 0x00000000; /* transparent */ - else if (xorPixel == 0xFFFFFFFF) /* white */ + else if (xorPixelMasked == 0xFFFFFFFF) /* white */ *pDstPixel++ = 0xFF000000; /* inverted (set as black) */ else *pDstPixel++ = xorPixel;