From 4b80c46ae4ffae99f7e93441bcd3f05c1b032248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Wed, 2 Jul 2014 14:51:46 -0400 Subject: [PATCH] libfreerdp-codec: improve ClearCodec robustness --- include/freerdp/codec/nsc.h | 2 +- libfreerdp/codec/clear.c | 166 ++++++++++++++++++------------------ libfreerdp/codec/nsc.c | 8 +- 3 files changed, 92 insertions(+), 84 deletions(-) diff --git a/include/freerdp/codec/nsc.h b/include/freerdp/codec/nsc.h index a5832352e..ebd5a416b 100644 --- a/include/freerdp/codec/nsc.h +++ b/include/freerdp/codec/nsc.h @@ -84,7 +84,7 @@ struct _NSC_CONTEXT FREERDP_API NSC_CONTEXT* nsc_context_new(void); FREERDP_API void nsc_context_set_pixel_format(NSC_CONTEXT* context, RDP_PIXEL_FORMAT pixel_format); -FREERDP_API void nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, +FREERDP_API int nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, UINT16 width, UINT16 height, BYTE* data, UINT32 length); FREERDP_API void nsc_compose_message(NSC_CONTEXT* context, wStream* s, BYTE* bmpdata, int width, int height, int rowstride); diff --git a/libfreerdp/codec/clear.c b/libfreerdp/codec/clear.c index 10e13e762..15e62191a 100644 --- a/libfreerdp/codec/clear.c +++ b/libfreerdp/codec/clear.c @@ -72,15 +72,14 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, UINT32 residualByteCount; UINT32 bandsByteCount; UINT32 subcodecByteCount; - BYTE runLengthFactor1; - UINT16 runLengthFactor2; - UINT32 runLengthFactor3; UINT32 runLengthFactor; UINT32 pixelX, pixelY; UINT32 pixelIndex = 0; UINT32 pixelCount = 0; - UINT32* pSrcPixel = NULL; - UINT32* pDstPixel = NULL; + BYTE* pSrcPixel8 = NULL; + BYTE* pDstPixel8 = NULL; + UINT32* pSrcPixel32 = NULL; + UINT32* pDstPixel32 = NULL; if (!ppDstData) return -1001; @@ -135,12 +134,14 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, return -1008; nSrcStep = nWidth * 4; + pSrcPixel8 = glyphData; + pDstPixel8 = &pDstData[(nYDst * nDstStep) + (nXDst * 4)]; for (y = 0; y < nHeight; y++) { - pSrcPixel = (UINT32*) &glyphData[y * nSrcStep]; - pDstPixel = (UINT32*) &pDstData[((nYDst + y) * nDstStep) + (nXDst * 4)]; - CopyMemory(pDstPixel, pSrcPixel, nSrcStep); + CopyMemory(pDstPixel8, pSrcPixel8, nSrcStep); + pSrcPixel8 += nSrcStep; + pDstPixel8 += nDstStep; } return 1; /* Finish */ @@ -181,26 +182,23 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, color = RGB32(residualData[suboffset + 2], residualData[suboffset + 1], residualData[suboffset + 0]); suboffset += 3; - runLengthFactor1 = residualData[suboffset]; - runLengthFactor = runLengthFactor1; - suboffset += 1; + runLengthFactor = (UINT32) residualData[suboffset]; + suboffset++; - if (runLengthFactor1 >= 0xFF) + if (runLengthFactor >= 0xFF) { if ((residualByteCount - suboffset) < 2) return -1012; - runLengthFactor2 = *((UINT16*) &residualData[suboffset]); - runLengthFactor = runLengthFactor2; + runLengthFactor = (UINT32) *((UINT16*) &residualData[suboffset]); suboffset += 2; - if (runLengthFactor2 >= 0xFFFF) + if (runLengthFactor >= 0xFFFF) { if ((residualByteCount - suboffset) < 4) return -1013; - runLengthFactor3 = *((UINT32*) &residualData[suboffset]); - runLengthFactor = runLengthFactor3; + runLengthFactor = *((UINT32*) &residualData[suboffset]); suboffset += 4; } } @@ -216,12 +214,12 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, if (count > pixelCount) count = pixelCount; - pDstPixel = (UINT32*) &pDstData[((nYDst + pixelY) * nDstStep) + ((nXDst + pixelX) * 4)]; + pDstPixel32 = (UINT32*) &pDstData[((nYDst + pixelY) * nDstStep) + ((nXDst + pixelX) * 4)]; pixelCount -= count; while (count--) { - *pDstPixel++ = color; + *pDstPixel32++ = color; } pixelX = 0; @@ -305,7 +303,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarPixelCount = (yEnd - yStart + 1); if (vBarPixelCount > 52) - return -1; + return -1020; if ((vBarHeader & 0xC000) == 0x8000) /* VBAR_CACHE_HIT */ { @@ -315,7 +313,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, // vBarIndex, clear->VBarStorageCursor, clear->ShortVBarStorageCursor); if (vBarIndex >= 32768) - return -1020; + return -1021; vBarEntry = &(clear->VBarStorage[vBarIndex]); } @@ -324,10 +322,10 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarIndex = (vBarHeader & 0x3FFF); if (vBarIndex >= 16384) - return -1021; + return -1022; if ((bandsByteCount - suboffset) < 1) - return -1022; + return -1023; vBarYOn = vBar[2]; suboffset += 1; @@ -340,7 +338,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarShortEntry = &(clear->ShortVBarStorage[clear->ShortVBarStorageCursor]); if (!vBarShortEntry) - return -1023; + return -1024; vBarShortPixelCount = vBarShortEntry->count; @@ -352,19 +350,19 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarYOff = ((vBarHeader >> 8) & 0x3F); if (vBarYOff < vBarYOn) - return -1024; + return -1025; vBarPixels = &vBar[2]; vBarShortPixelCount = (vBarYOff - vBarYOn); if (vBarShortPixelCount > 52) - return -1; + return -1026; //printf("SHORT_VBAR_CACHE_MISS: vBarYOn: %d vBarYOff: %d vBarPixelCount: %d Cursor: %d / %d\n", // vBarYOn, vBarYOff, vBarShortPixelCount, clear->VBarStorageCursor, clear->ShortVBarStorageCursor); if ((bandsByteCount - suboffset) < (vBarShortPixelCount * 3)) - return -1025; + return -1027; vBarShortEntry = &(clear->ShortVBarStorage[clear->ShortVBarStorageCursor]); @@ -372,15 +370,15 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarShortEntry->pixels = (UINT32*) malloc(52 * 4); if (!vBarShortEntry->pixels) - return -1026; + return -1028; - pDstPixel = vBarShortEntry->pixels; + pDstPixel32 = vBarShortEntry->pixels; for (y = 0; y < vBarShortPixelCount; y++) { - *pDstPixel = RGB32(vBarPixels[2], vBarPixels[1], vBarPixels[0]); + *pDstPixel32 = RGB32(vBarPixels[2], vBarPixels[1], vBarPixels[0]); vBarPixels += 3; - pDstPixel++; + pDstPixel32++; } suboffset += (vBarShortPixelCount * 3); @@ -392,7 +390,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, } else { - return -1027; /* invalid vBarHeader */ + return -1029; /* invalid vBarHeader */ } if (vBarUpdate) @@ -403,9 +401,9 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, vBarEntry->pixels = (UINT32*) malloc(52 * 4); if (!vBarEntry->pixels) - return -1028; + return -1030; - pDstPixel = vBarEntry->pixels; + pDstPixel32 = vBarEntry->pixels; /* if (y < vBarYOn), use colorBkg */ @@ -417,8 +415,8 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, while (count--) { - *pDstPixel = colorBkg; - pDstPixel++; + *pDstPixel32 = colorBkg; + pDstPixel32++; } /* @@ -432,9 +430,9 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, if ((y + count) > vBarPixelCount) count = (vBarPixelCount > y) ? (vBarPixelCount - y) : 0; - pSrcPixel = &(vBarShortEntry->pixels[y - vBarYOn]); - CopyMemory(pDstPixel, pSrcPixel, count * 4); - pDstPixel += count; + pSrcPixel32 = &(vBarShortEntry->pixels[y - vBarYOn]); + CopyMemory(pDstPixel32, pSrcPixel32, count * 4); + pDstPixel32 += count; /* if (y >= (vBarYOn + vBarShortPixelCount)), use colorBkg */ @@ -443,8 +441,8 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, while (count--) { - *pDstPixel = colorBkg; - pDstPixel++; + *pDstPixel32 = colorBkg; + pDstPixel32++; } vBarEntry->count = vBarPixelCount; @@ -455,14 +453,14 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, nXDstRel = nXDst + xStart; nYDstRel = nYDst + yStart; - pSrcPixel = (UINT32*) vBarEntry->pixels; - pDstPixel = (UINT32*) &pDstData[(nYDstRel * nDstStep) + ((nXDstRel + i) * 4)]; + pSrcPixel32 = (UINT32*) vBarEntry->pixels; + pDstPixel8 = &pDstData[(nYDstRel * nDstStep) + ((nXDstRel + i) * 4)]; for (y = 0; y < count; y++) { - *pDstPixel = *pSrcPixel; - pDstPixel = (UINT32*) (((BYTE*) pDstPixel) + nDstStep); - pSrcPixel++; + *((UINT32*) pDstPixel8) = *pSrcPixel32; + pDstPixel8 += nDstStep; + pSrcPixel32++; } } } @@ -482,10 +480,9 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, BYTE subcodecId; BYTE* subcodecs; UINT32 suboffset; - BYTE* pixels; if ((SrcSize - offset) < subcodecByteCount) - return -1029; + return -1031; suboffset = 0; subcodecs = &pSrcData[offset]; @@ -493,7 +490,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, while (suboffset < subcodecByteCount) { if ((subcodecByteCount - suboffset) < 13) - return -1030; + return -1032; xStart = *((UINT16*) &subcodecs[suboffset]); yStart = *((UINT16*) &subcodecs[suboffset + 2]); @@ -507,7 +504,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, // bitmapDataByteCount, subcodecByteCount, suboffset, subcodecId); if ((subcodecByteCount - suboffset) < bitmapDataByteCount) - return -1031; + return -1033; nXDstRel = nXDst + xStart; nYDstRel = nYDst + yStart; @@ -517,34 +514,36 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, if (subcodecId == 0) /* Uncompressed */ { if (bitmapDataByteCount != (width * height * 3)) - return -1032; + return -1034; - pixels = bitmapData; + pSrcPixel8 = bitmapData; for (y = 0; y < height; y++) { - pDstPixel = (UINT32*) &pDstData[((nYDstRel + y) * nDstStep) + (nXDstRel * 4)]; + pDstPixel32 = (UINT32*) &pDstData[((nYDstRel + y) * nDstStep) + (nXDstRel * 4)]; for (x = 0; x < width; x++) { - *pDstPixel = RGB32(pixels[2], pixels[1], pixels[0]); - pixels += 3; - pDstPixel++; + *pDstPixel32 = RGB32(pSrcPixel8[2], pSrcPixel8[1], pSrcPixel8[0]); + pSrcPixel8 += 3; + pDstPixel32++; } } } else if (subcodecId == 1) /* NSCodec */ { - nsc_process_message(clear->nsc, 32, width, height, bitmapData, bitmapDataByteCount); + if (nsc_process_message(clear->nsc, 32, width, height, bitmapData, bitmapDataByteCount) < 0) + return -1035; nSrcStep = width * 4; - pixels = clear->nsc->BitmapData; + pSrcPixel8 = clear->nsc->BitmapData; + pDstPixel8 = &pDstData[(nYDstRel * nDstStep) + (nXDstRel * 4)]; for (y = 0; y < height; y++) { - pSrcPixel = (UINT32*) &pixels[y * nSrcStep]; - pDstPixel = (UINT32*) &pDstData[((nYDstRel + y) * nDstStep) + (nXDstRel * 4)]; - CopyMemory(pDstPixel, pSrcPixel, nSrcStep); + CopyMemory(pDstPixel8, pSrcPixel8, nSrcStep); + pSrcPixel8 += nSrcStep; + pDstPixel8 += nDstStep; } } else if (subcodecId == 2) /* CLEARCODEC_SUBCODEC_RLEX */ @@ -576,51 +575,51 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, while (bitmapDataOffset < bitmapDataByteCount) { + if ((bitmapDataByteCount - bitmapDataOffset) < 2) + return -1; + stopIndex = bitmapData[bitmapDataOffset] & CLEAR_8BIT_MASKS[numBits]; suiteDepth = (bitmapData[bitmapDataOffset] >> numBits) & CLEAR_8BIT_MASKS[numBits]; startIndex = stopIndex - suiteDepth; bitmapDataOffset++; - runLengthFactor1 = bitmapData[bitmapDataOffset]; - runLengthFactor = runLengthFactor1; - bitmapDataOffset += 1; + runLengthFactor = (UINT32) bitmapData[bitmapDataOffset]; + bitmapDataOffset++; - if (runLengthFactor1 >= 0xFF) + if (runLengthFactor >= 0xFF) { if ((bitmapDataByteCount - bitmapDataOffset) < 2) - return -1033; + return -1036; - runLengthFactor2 = *((UINT16*) &bitmapData[bitmapDataOffset]); - runLengthFactor = runLengthFactor2; + runLengthFactor = (UINT32) *((UINT16*) &bitmapData[bitmapDataOffset]); bitmapDataOffset += 2; - if (runLengthFactor2 >= 0xFFFF) + if (runLengthFactor >= 0xFFFF) { if ((bitmapDataByteCount - bitmapDataOffset) < 4) return -1034; - runLengthFactor3 = *((UINT32*) &bitmapData[bitmapDataOffset]); - runLengthFactor = runLengthFactor3; + runLengthFactor = *((UINT32*) &bitmapData[bitmapDataOffset]); bitmapDataOffset += 4; } } if (startIndex > paletteCount) - return -1034; + return -1037; if (stopIndex > paletteCount) - return -1035; + return -1038; suiteIndex = startIndex; for (y = 0; y < height; y++) { - pDstPixel = (UINT32*) &pDstData[((nYDstRel + y) * nDstStep) + (nXDstRel * 4)]; + pDstPixel32 = (UINT32*) &pDstData[((nYDstRel + y) * nDstStep) + (nXDstRel * 4)]; for (x = 0; x < width; x++) { - *pDstPixel = palette[suiteIndex]; - pDstPixel++; + *pDstPixel32 = palette[suiteIndex]; + pDstPixel32++; if (runLengthFactor) { @@ -639,7 +638,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, } else { - return -1036; + return -1039; } suboffset += bitmapDataByteCount; @@ -661,19 +660,22 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize, glyphData = clear->GlyphCache[glyphIndex]; if (!glyphData) - return -1037; + return -1040; nSrcStep = nWidth * 4; + pDstPixel8 = glyphData; + pSrcPixel8 = &pDstData[(nYDst * nDstStep) + (nXDst * 4)]; + for (y = 0; y < nHeight; y++) { - pDstPixel = (UINT32*) &glyphData[y * nSrcStep]; - pSrcPixel = (UINT32*) &pDstData[((nYDst + y) * nDstStep) + (nXDst * 4)]; - CopyMemory(pDstPixel, pSrcPixel, nSrcStep); + CopyMemory(pDstPixel8, pSrcPixel8, nSrcStep); + pDstPixel8 += nSrcStep; + pSrcPixel8 += nDstStep; } } if (offset != SrcSize) - return -1038; + return -1041; return 1; } diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c index f04bac779..3379e5a49 100644 --- a/libfreerdp/codec/nsc.c +++ b/libfreerdp/codec/nsc.c @@ -355,11 +355,15 @@ void nsc_context_set_pixel_format(NSC_CONTEXT* context, RDP_PIXEL_FORMAT pixel_f } } -void nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, UINT16 width, UINT16 height, BYTE* data, UINT32 length) +int nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, UINT16 width, UINT16 height, BYTE* data, UINT32 length) { wStream* s; s = Stream_New(data, length); + + if (!s) + return -1; + context->bpp = bpp; context->width = width; context->height = height; @@ -375,4 +379,6 @@ void nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, UINT16 width, UINT16 PROFILER_ENTER(context->priv->prof_nsc_decode); context->decode(context); PROFILER_EXIT(context->priv->prof_nsc_decode); + + return 1; }