diff options
author | Caolán McNamara <caolanm@redhat.com> | 2015-09-16 14:17:35 +0100 |
---|---|---|
committer | Jan Holesovsky <kendy@collabora.com> | 2016-05-16 14:56:57 +0200 |
commit | 6e07820d4074b71e3b9caf2c491921eb678eec4b (patch) | |
tree | 282880f067dc2dea79542595d0d3fe28919d9795 | |
parent | 989592293474448af8458f924fa7f74d34955b60 (diff) |
make CalcMaskShift a verifiable operation
and check it on untrusted data
Change-Id: I7c97a27d70f91b9686adf9dcb8b68c5aa25c2b4c
Reviewed-on: https://gerrit.libreoffice.org/18637
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>
-rw-r--r-- | include/vcl/salbtype.hxx | 49 | ||||
-rw-r--r-- | vcl/opengl/salbmp.cxx | 66 | ||||
-rw-r--r-- | vcl/qa/cppunit/graphicfilter/data/bmp/pass/crash-1.bmp | bin | 0 -> 632 bytes | |||
-rw-r--r-- | vcl/quartz/salbmp.cxx | 46 | ||||
-rw-r--r-- | vcl/source/gdi/dibtools.cxx | 24 | ||||
-rw-r--r-- | vcl/unx/generic/gdi/salbmp.cxx | 37 | ||||
-rw-r--r-- | vcl/win/source/gdi/salbmp.cxx | 30 |
7 files changed, 173 insertions, 79 deletions
diff --git a/include/vcl/salbtype.hxx b/include/vcl/salbtype.hxx index ed6e5e06dce7..c02fee5f7986 100644 --- a/include/vcl/salbtype.hxx +++ b/include/vcl/salbtype.hxx @@ -195,29 +195,29 @@ struct VCL_DLLPUBLIC ColorMaskElement int mnShift; int mnOrShift; sal_uInt8 mnOr; - ColorMaskElement(sal_uInt32 nMask) + explicit ColorMaskElement(sal_uInt32 nMask = 0) : mnMask(nMask) , mnShift(0) , mnOrShift(0) , mnOr(0) { } - static bool CalcMaskShift(ColorMaskElement &rElem) + bool CalcMaskShift() { - if (rElem.mnMask == 0) + if (mnMask == 0) return true; // from which bit starts the mask? int nShift = 31; - while( nShift >= 0 && !( rElem.mnMask & ( 1 << nShift ) ) ) + while( nShift >= 0 && !( mnMask & ( 1 << nShift ) ) ) --nShift; - rElem.mnShift = nShift - 7; + mnShift = nShift - 7; int nLen = 0; // XXX determine number of bits set => walk right until null - while( nShift >= 0 && ( rElem.mnMask & ( 1 << nShift ) ) ) + while( nShift >= 0 && ( mnMask & ( 1 << nShift ) ) ) { nShift--; nLen++; @@ -226,8 +226,8 @@ struct VCL_DLLPUBLIC ColorMaskElement if (nLen > 8) // mask length must be 8 bits or less return false; - rElem.mnOrShift = 8 - nLen; - rElem.mnOr = static_cast<sal_uInt8>( ( 0xFF >> nLen ) << rElem.mnOrShift ); + mnOrShift = 8 - nLen; + mnOr = static_cast<sal_uInt8>( ( 0xFF >> nLen ) << mnOrShift ); return true; } @@ -243,11 +243,18 @@ class VCL_DLLPUBLIC ColorMask public: - inline ColorMask( sal_uLong nRedMask = 0UL, - sal_uLong nGreenMask = 0UL, - sal_uLong nBlueMask = 0UL, - sal_uLong nAlphaChannel = 0UL ); - inline ~ColorMask() {} + ColorMask(const ColorMaskElement& rRedMask = ColorMaskElement(), + const ColorMaskElement& rGreenMask = ColorMaskElement(), + const ColorMaskElement& rBlueMask = ColorMaskElement(), + sal_uInt32 nAlphaChannel = 0) + : maR(rRedMask) + , maG(rGreenMask) + , maB(rBlueMask) + , mnAlphaChannel(nAlphaChannel) + { + } + + ~ColorMask() {} inline sal_uLong GetRedMask() const; inline sal_uLong GetGreenMask() const; @@ -646,21 +653,7 @@ inline sal_uInt16 BitmapPalette::GetBestIndex( const BitmapColor& rCol ) const return nRetIndex; } -inline ColorMask::ColorMask( sal_uInt32 nRedMask, - sal_uInt32 nGreenMask, - sal_uInt32 nBlueMask, - sal_uInt32 nAlphaChannel ) - : maR(nRedMask) - , maG(nGreenMask) - , maB(nBlueMask) - , mnAlphaChannel(nAlphaChannel) -{ - ColorMaskElement::CalcMaskShift(maR); - ColorMaskElement::CalcMaskShift(maG); - ColorMaskElement::CalcMaskShift(maB); -} - -inline sal_uLong ColorMask::GetRedMask() const +inline sal_uInt32 ColorMask::GetRedMask() const { return maR.mnMask; } diff --git a/vcl/opengl/salbmp.cxx b/vcl/opengl/salbmp.cxx index ce03ccfe98f2..34635a14c9c5 100644 --- a/vcl/opengl/salbmp.cxx +++ b/vcl/opengl/salbmp.cxx @@ -729,36 +729,44 @@ BitmapBuffer* OpenGLSalBitmap::AcquireBuffer( BitmapAccessMode nMode ) pBuffer->mnScanlineSize = mnBytesPerRow; pBuffer->mpBits = maUserBuffer.get(); pBuffer->mnBitCount = mnBits; - switch( mnBits ) + switch (mnBits) { - case 1: pBuffer->mnFormat = BMP_FORMAT_1BIT_MSB_PAL; break; - case 4: pBuffer->mnFormat = BMP_FORMAT_4BIT_MSN_PAL; break; - case 8: pBuffer->mnFormat = BMP_FORMAT_8BIT_PAL; break; - case 16: -#ifdef WNT - pBuffer->mnFormat = BMP_FORMAT_16BIT_TC_MSB_MASK; - pBuffer->maColorMask = ColorMask(0x7c00, 0x03e0, 0x001f); -#else - pBuffer->mnFormat = BMP_FORMAT_16BIT_TC_MSB_MASK; - pBuffer->maColorMask = ColorMask(0xf800, 0x07e0, 0x001f); -#endif - break; - case 24: -#ifdef WNT - pBuffer->mnFormat = BMP_FORMAT_24BIT_TC_BGR; -#else - pBuffer->mnFormat = BMP_FORMAT_24BIT_TC_RGB; -#endif - break; - case 32: -#ifdef WNT - pBuffer->mnFormat = BMP_FORMAT_32BIT_TC_BGRA; - pBuffer->maColorMask = ColorMask(0x00ff0000, 0x0000ff00, 0x000000ff); -#else - pBuffer->mnFormat = BMP_FORMAT_32BIT_TC_RGBA; - pBuffer->maColorMask = ColorMask(0xff000000, 0x00ff0000, 0x0000ff00); -#endif - break; + case 1: + pBuffer->mnFormat = BMP_FORMAT_1BIT_MSB_PAL; + break; + case 4: + pBuffer->mnFormat = BMP_FORMAT_4BIT_MSN_PAL; + break; + case 8: + pBuffer->mnFormat = BMP_FORMAT_8BIT_PAL; + break; + case 16: + { + pBuffer->mnFormat = BMP_FORMAT_16BIT_TC_MSB_MASK; + ColorMaskElement aRedMask(0xf800); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x07e0); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x001f); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + break; + } + case 24: + pBuffer->mnFormat = BMP_FORMAT_24BIT_TC_RGB; + break; + case 32: + { + pBuffer->mnFormat = BMP_FORMAT_32BIT_TC_RGBA; + ColorMaskElement aRedMask(0xff000000); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x00ff0000); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x0000ff00); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + break; + } } return pBuffer; diff --git a/vcl/qa/cppunit/graphicfilter/data/bmp/pass/crash-1.bmp b/vcl/qa/cppunit/graphicfilter/data/bmp/pass/crash-1.bmp Binary files differnew file mode 100644 index 000000000000..84b6c35c87e6 --- /dev/null +++ b/vcl/qa/cppunit/graphicfilter/data/bmp/pass/crash-1.bmp diff --git a/vcl/quartz/salbmp.cxx b/vcl/quartz/salbmp.cxx index 06b55ac786a9..8c05de3f6b85 100644 --- a/vcl/quartz/salbmp.cxx +++ b/vcl/quartz/salbmp.cxx @@ -726,16 +726,42 @@ BitmapBuffer* QuartzSalBitmap::AcquireBuffer( BitmapAccessMode /*nMode*/ ) pBuffer->mnBitCount = mnBits; switch( mnBits ) { - case 1: pBuffer->mnFormat = BMP_FORMAT_1BIT_MSB_PAL; break; - case 4: pBuffer->mnFormat = BMP_FORMAT_4BIT_MSN_PAL; break; - case 8: pBuffer->mnFormat = BMP_FORMAT_8BIT_PAL; break; - case 16: pBuffer->mnFormat = BMP_FORMAT_16BIT_TC_MSB_MASK; - pBuffer->maColorMask = ColorMask( k16BitRedColorMask, k16BitGreenColorMask, k16BitBlueColorMask ); - break; - case 24: pBuffer->mnFormat = BMP_FORMAT_24BIT_TC_BGR; break; - case 32: pBuffer->mnFormat = BMP_FORMAT_32BIT_TC_ARGB; - pBuffer->maColorMask = ColorMask( k32BitRedColorMask, k32BitGreenColorMask, k32BitBlueColorMask ); - break; + case 1: + pBuffer->mnFormat = BMP_FORMAT_1BIT_MSB_PAL; + break; + case 4: + pBuffer->mnFormat = BMP_FORMAT_4BIT_MSN_PAL; + break; + case 8: + pBuffer->mnFormat = BMP_FORMAT_8BIT_PAL; + break; + case 16: + { + pBuffer->mnFormat = BMP_FORMAT_16BIT_TC_MSB_MASK; + ColorMaskElement aRedMask(k16BitRedColorMask); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(k16BitGreenColorMask); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(k16BitBlueColorMask); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + break; + } + case 24: + pBuffer->mnFormat = BMP_FORMAT_24BIT_TC_BGR; + break; + case 32: + { + pBuffer->mnFormat = BMP_FORMAT_32BIT_TC_ARGB; + ColorMaskElement aRedMask(k32BitRedColorMask); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(k32BitGreenColorMask); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(k32BitBlueColorMask); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + break; + } } pBuffer->mnFormat |= BMP_FORMAT_BOTTOM_UP; diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index 509d2e45f573..ff7d7d186b09 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -608,7 +608,17 @@ bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& r case( 16 ): { - ColorMask aMask( nRMask, nGMask, nBMask ); + ColorMaskElement aRedMask(nRMask); + if (!aRedMask.CalcMaskShift()) + return false; + ColorMaskElement aGreenMask(nGMask); + if (!aGreenMask.CalcMaskShift()) + return false; + ColorMaskElement aBlueMask(nBMask); + if (!aBlueMask.CalcMaskShift()) + return false; + + ColorMask aMask(aRedMask, aGreenMask, aBlueMask); BitmapColor aColor; sal_uInt16* pTmp16; @@ -655,7 +665,17 @@ bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& r case( 32 ): { - ColorMask aMask(nRMask, nGMask, nBMask); + ColorMaskElement aRedMask(nRMask); + if (!aRedMask.CalcMaskShift()) + return false; + ColorMaskElement aGreenMask(nGMask); + if (!aGreenMask.CalcMaskShift()) + return false; + ColorMaskElement aBlueMask(nBMask); + if (!aBlueMask.CalcMaskShift()) + return false; + ColorMask aMask(aRedMask, aGreenMask, aBlueMask); + BitmapColor aColor; sal_uInt32* pTmp32; diff --git a/vcl/unx/generic/gdi/salbmp.cxx b/vcl/unx/generic/gdi/salbmp.cxx index 8087b2245be0..2dce489df249 100644 --- a/vcl/unx/generic/gdi/salbmp.cxx +++ b/vcl/unx/generic/gdi/salbmp.cxx @@ -155,14 +155,30 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB( case( 8 ): pDIB->mnFormat |= BMP_FORMAT_8BIT_PAL; break; #ifdef OSL_BIGENDIAN case(16 ): + { pDIB->mnFormat|= BMP_FORMAT_16BIT_TC_MSB_MASK; - pDIB->maColorMask = ColorMask( 0xf800, 0x07e0, 0x001f ); + ColorMaskElement aRedMask(0xf800); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x07e0); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x001f); + aBlueMask.CalcMaskShift(); + pDIB->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); break; + } #else case(16 ): + { pDIB->mnFormat|= BMP_FORMAT_16BIT_TC_LSB_MASK; - pDIB->maColorMask = ColorMask( 0xf800, 0x07e0, 0x001f ); + ColorMaskElement aRedMask(0xf800); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x07e0); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x001f); + aBlueMask.CalcMaskShift(); + pDIB->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); break; + } #endif default: nBitCount = 24; @@ -279,8 +295,13 @@ BitmapBuffer* X11SalBitmap::ImplCreateDIB( case( 16 ): { - nDstFormat |= BMP_FORMAT_24BIT_TC_BGR; - aSrcBuf.maColorMask = ColorMask( pImage->red_mask, pImage->green_mask, pImage->blue_mask ); + ColorMaskElement aRedMask(pImage->red_mask); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(pImage->green_mask); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(pImage->blue_mask); + aBlueMask.CalcMaskShift(); + aSrcBuf.maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); if( LSBFirst == pImage->byte_order ) { @@ -454,7 +475,13 @@ XImage* X11SalBitmap::ImplCreateXImage( #endif - xMask.reset(new ColorMask( pImage->red_mask, pImage->green_mask, pImage->blue_mask )); + ColorMaskElement aRedMask(pImage->red_mask); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(pImage->green_mask); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(pImage->blue_mask); + aBlueMask.CalcMaskShift(); + xMask.reset(new ColorMask(aRedMask, aGreenMask, aBlueMask)); } break; diff --git a/vcl/win/source/gdi/salbmp.cxx b/vcl/win/source/gdi/salbmp.cxx index 86656dd1d86a..5612b6d6fc5a 100644 --- a/vcl/win/source/gdi/salbmp.cxx +++ b/vcl/win/source/gdi/salbmp.cxx @@ -898,14 +898,34 @@ BitmapBuffer* WinSalBitmap::AcquireBuffer( BitmapAccessMode /*nMode*/ ) if( pBIH->biCompression == BI_BITFIELDS ) { nOffset = 3 * sizeof( RGBQUAD ); - pBuffer->maColorMask = ColorMask( *(UINT32*) &pBI->bmiColors[ 0 ], - *(UINT32*) &pBI->bmiColors[ 1 ], - *(UINT32*) &pBI->bmiColors[ 2 ] ); + ColorMaskElement aRedMask(*(UINT32*) &pBI->bmiColors[ 0 ]); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(*(UINT32*) &pBI->bmiColors[ 1 ]); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(*(UINT32*) &pBI->bmiColors[ 2 ]); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); } else if( pBIH->biBitCount == 16 ) - pBuffer->maColorMask = ColorMask( 0x00007c00UL, 0x000003e0UL, 0x0000001fUL ); + { + ColorMaskElement aRedMask(0x00007c00UL); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x000003e0UL); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x0000001fUL); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + } else - pBuffer->maColorMask = ColorMask( 0x00ff0000UL, 0x0000ff00UL, 0x000000ffUL ); + { + ColorMaskElement aRedMask(0x00ff0000UL); + aRedMask.CalcMaskShift(); + ColorMaskElement aGreenMask(0x0000ff00UL); + aGreenMask.CalcMaskShift(); + ColorMaskElement aBlueMask(0x000000ffUL); + aBlueMask.CalcMaskShift(); + pBuffer->maColorMask = ColorMask(aRedMask, aGreenMask, aBlueMask); + } pBuffer->mpBits = (PBYTE) pBI + *(DWORD*) pBI + nOffset; } |