vcl/source/gdi/dibtools.cxx | 175 +++++++++++++++++++++++++------------------- 1 file changed, 103 insertions(+), 72 deletions(-)
New commits: commit fdcc9e0b8f083d9964d195ebcddcdeedd40b871d Author: David Tardon <dtar...@redhat.com> Date: Sun Jan 24 14:32:23 2016 +0100 limit variable scope Change-Id: I98d281f55ad76930ccc3fb768fe87aef0c55d2c7 diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index 2ec4c2a..cfc31e9 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -778,7 +778,6 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon { sal_uInt32 nCodedSize(0); sal_uInt32 nUncodedSize(0); - sal_uLong nCodedPos(0); // read coding information rIStm.ReadUInt32( nCodedSize ).ReadUInt32( nUncodedSize ).ReadUInt32( aHeader.nCompression ); @@ -791,7 +790,7 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon if (nSizeInc > 0) { // decode buffer - nCodedPos = rIStm.Tell(); + const sal_uLong nCodedPos = rIStm.Tell(); ZCodec aCodec; aCodec.BeginCompression(); aData.resize(nSizeInc); commit 93ca0057d6eca140764de446ba9b7d4128e88205 Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 19:25:12 2016 +0100 move bmp dimension sanity check to a better place Change-Id: I0eb67fedb14d9847417f1fef74d6837bfa672ae1 diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index 041a7e3..2ec4c2a 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -301,17 +301,6 @@ bool ImplReadDIBInfoHeader(SvStream& rIStm, DIBV5Header& rHeader, bool& bTopDown return false; } - if (rHeader.nCompression == 0) - { - sal_uInt64 nMaxSize = rIStm.remainingSize(); - if (rHeader.nHeight != 0) - nMaxSize /= rHeader.nHeight; - if (rHeader.nPlanes != 0) - nMaxSize /= rHeader.nPlanes; - if (sal_Int64(nMaxSize) < rHeader.nWidth) - return false; - } - return rIStm.good(); } @@ -480,13 +469,8 @@ bool ImplDecodeRLE( sal_uInt8* pBuffer, DIBV5Header& rHeader, BitmapWriteAccess& return true; } -bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& rAcc, BitmapWriteAccess* pAccAlpha, bool bTopDown, bool& rAlphaUsed) +bool ImplReadDIBBits(SvStream& rIStm, DIBV5Header& rHeader, BitmapWriteAccess& rAcc, BitmapWriteAccess* pAccAlpha, bool bTopDown, bool& rAlphaUsed, const sal_uInt64 nAlignedWidth) { - const sal_Int64 nBitsPerLine (static_cast<sal_Int64>(rHeader.nWidth) * static_cast<sal_Int64>(rHeader.nBitCount)); - if (nBitsPerLine > SAL_MAX_UINT32) - return false; - - const sal_uLong nAlignedWidth = AlignedWidth4Bytes(static_cast<sal_uLong>(nBitsPerLine)); sal_uInt32 nRMask(( rHeader.nBitCount == 16 ) ? 0x00007c00UL : 0x00ff0000UL); sal_uInt32 nGMask(( rHeader.nBitCount == 16 ) ? 0x000003e0UL : 0x0000ff00UL); sal_uInt32 nBMask(( rHeader.nBitCount == 16 ) ? 0x0000001fUL : 0x000000ffUL); @@ -860,6 +844,20 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon pIStm = &rIStm; } + const sal_Int64 nBitsPerLine (static_cast<sal_Int64>(aHeader.nWidth) * static_cast<sal_Int64>(aHeader.nBitCount)); + const sal_uInt64 nAlignedWidth(AlignedWidth4Bytes(static_cast<sal_uLong>(nBitsPerLine))); + bRet = nBitsPerLine <= SAL_MAX_UINT32; + + // (partially) check the image dimensions to avoid potential large bitmap allocation if the input is damaged + if (aHeader.nCompression == ZCOMPRESS || aHeader.nCompression == COMPRESS_NONE) + { + sal_uInt64 nMaxWidth = rIStm.remainingSize(); + if (aHeader.nHeight != 0) + nMaxWidth /= aHeader.nHeight; + if (nMaxWidth < nAlignedWidth) + return false; + } + const Size aSizePixel(aHeader.nWidth, aHeader.nHeight); BitmapPalette aDummyPal; Bitmap aNewBmp(aSizePixel, nBitCount, &aDummyPal); @@ -896,6 +894,7 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon aNewBmpAlpha = Bitmap(aSizePixel, 8); pAccAlpha = aNewBmpAlpha.AcquireWriteAccess(); } + // read palette if(nColors) { @@ -913,7 +912,7 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon pIStm->SeekRel(nOffset - (pIStm->Tell() - nStmPos)); } - bRet = ImplReadDIBBits(*pIStm, aHeader, *pAcc, pAccAlpha, bTopDown, bAlphaUsed); + bRet = ImplReadDIBBits(*pIStm, aHeader, *pAcc, pAccAlpha, bTopDown, bAlphaUsed, nAlignedWidth); if(bRet && aHeader.nXPelsPerMeter && aHeader.nYPelsPerMeter) { commit eae3881b92ece4ce7ad93dd836e396b0ad44d16b Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 19:18:47 2016 +0100 limit variable scope Change-Id: I961d1378f81b511be3173c61206b53983841abbe diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index 8160172..041a7e3 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -772,42 +772,6 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon } const sal_uInt16 nBitCount(discretizeBitcount(aHeader.nBitCount)); - const Size aSizePixel(aHeader.nWidth, aHeader.nHeight); - BitmapPalette aDummyPal; - Bitmap aNewBmp(aSizePixel, nBitCount, &aDummyPal); - BitmapWriteAccess* pAcc = aNewBmp.AcquireWriteAccess(); - if (!pAcc) - return false; - if (pAcc->Width() != aHeader.nWidth || pAcc->Height() != aHeader.nHeight) - { - Bitmap::ReleaseAccess(pAcc); - return false; - } - Bitmap aNewBmpAlpha; - BitmapWriteAccess* pAccAlpha = nullptr; - bool bAlphaPossible(pBmpAlpha && aHeader.nBitCount == 32); - - if (bAlphaPossible) - { - const bool bRedSet(0 != aHeader.nV5RedMask); - const bool bGreenSet(0 != aHeader.nV5GreenMask); - const bool bBlueSet(0 != aHeader.nV5BlueMask); - - // some clipboard entries have alpha mask on zero to say that there is - // no alpha; do only use this when the other masks are set. The MS docu - // says that masks are only to be set when bV5Compression is set to - // BI_BITFIELDS, but there seem to exist a wild variety of usages... - if((bRedSet || bGreenSet || bBlueSet) && (0 == aHeader.nV5AlphaMask)) - { - bAlphaPossible = false; - } - } - - if (bAlphaPossible) - { - aNewBmpAlpha = Bitmap(aSizePixel, 8); - pAccAlpha = aNewBmpAlpha.AcquireWriteAccess(); - } sal_uInt16 nColors(0); SvStream* pIStm; @@ -896,6 +860,42 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon pIStm = &rIStm; } + const Size aSizePixel(aHeader.nWidth, aHeader.nHeight); + BitmapPalette aDummyPal; + Bitmap aNewBmp(aSizePixel, nBitCount, &aDummyPal); + BitmapWriteAccess* pAcc = aNewBmp.AcquireWriteAccess(); + if (!pAcc) + return false; + if (pAcc->Width() != aHeader.nWidth || pAcc->Height() != aHeader.nHeight) + { + Bitmap::ReleaseAccess(pAcc); + return false; + } + Bitmap aNewBmpAlpha; + BitmapWriteAccess* pAccAlpha = nullptr; + bool bAlphaPossible(pBmpAlpha && aHeader.nBitCount == 32); + + if (bAlphaPossible) + { + const bool bRedSet(0 != aHeader.nV5RedMask); + const bool bGreenSet(0 != aHeader.nV5GreenMask); + const bool bBlueSet(0 != aHeader.nV5BlueMask); + + // some clipboard entries have alpha mask on zero to say that there is + // no alpha; do only use this when the other masks are set. The MS docu + // says that masks are only to be set when bV5Compression is set to + // BI_BITFIELDS, but there seem to exist a wild variety of usages... + if((bRedSet || bGreenSet || bBlueSet) && (0 == aHeader.nV5AlphaMask)) + { + bAlphaPossible = false; + } + } + + if (bAlphaPossible) + { + aNewBmpAlpha = Bitmap(aSizePixel, 8); + pAccAlpha = aNewBmpAlpha.AcquireWriteAccess(); + } // read palette if(nColors) { commit 83cebdc81cf39c951e3d854433324d16fa0cba4d Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 18:54:45 2016 +0100 limit variable scope Change-Id: Ic78c1e437f680a24427e48523d26c89b309d1a32 diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index 360c577..8160172 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -828,7 +828,6 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon if(ZCOMPRESS == aHeader.nCompression) { - ZCodec aCodec; sal_uInt32 nCodedSize(0); sal_uInt32 nUncodedSize(0); sal_uLong nCodedPos(0); @@ -845,6 +844,7 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon { // decode buffer nCodedPos = rIStm.Tell(); + ZCodec aCodec; aCodec.BeginCompression(); aData.resize(nSizeInc); size_t nDataPos(0); commit 2f0cf9872644cb83a3125bb582a7773d8eea2cb6 Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 16:30:28 2016 +0100 sanitize input to avoid large allocation Also avoid use of zero-sized array. Change-Id: I843f6ffa7821b10676e590a5744b1cdc4864913b diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index e2767cc..360c577 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -812,7 +812,7 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon sal_uInt16 nColors(0); SvStream* pIStm; std::unique_ptr<SvMemoryStream> pMemStm; - sal_uInt8* pData = nullptr; + std::vector<sal_uInt8> aData; if (aHeader.nBitCount <= 8) { @@ -837,22 +837,58 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon rIStm.ReadUInt32( nCodedSize ).ReadUInt32( nUncodedSize ).ReadUInt32( aHeader.nCompression ); if (nCodedSize > rIStm.remainingSize()) nCodedSize = sal_uInt32(rIStm.remainingSize()); - pData = static_cast<sal_uInt8*>(rtl_allocateMemory( nUncodedSize )); + size_t nSizeInc(4 * rIStm.remainingSize()); + if (nUncodedSize < nSizeInc) + nSizeInc = nUncodedSize; - // decode buffer - nCodedPos = rIStm.Tell(); - aCodec.BeginCompression(); - aCodec.Read( rIStm, pData, nUncodedSize ); - aCodec.EndCompression(); - - // Seek behind the encoded block. There might have been bytes left or the codec might have read more than necessary. - rIStm.Seek(nCodedSize + nCodedPos); + if (nSizeInc > 0) + { + // decode buffer + nCodedPos = rIStm.Tell(); + aCodec.BeginCompression(); + aData.resize(nSizeInc); + size_t nDataPos(0); + while (nUncodedSize > nDataPos) + { + assert(aData.size() > nDataPos); + const size_t nToRead((std::min)(nUncodedSize - nDataPos, aData.size() - nDataPos)); + assert(nToRead > 0); + assert(!aData.empty()); + const long nRead = aCodec.Read(rIStm, &aData.front() + nDataPos, sal_uInt32(nToRead)); + if (nRead > 0) + { + nDataPos += static_cast<unsigned long>(nRead); + // we haven't read everything yet: resize buffer and continue + if (nDataPos < nUncodedSize) + aData.resize(aData.size() + nSizeInc); + } + else + { + break; + } + } + // truncate the data buffer to actually read size + aData.resize(nDataPos); + // set the real uncoded size + nUncodedSize = sal_uInt32(aData.size()); + aCodec.EndCompression(); + + // Seek behind the encoded block. There might have been bytes left or the codec might have read more than necessary. + rIStm.Seek(nCodedSize + nCodedPos); + } + else + { + // add something so we can take address of the first element + aData.resize(1); + nUncodedSize = 0; + } // set decoded bytes to memory stream, // from which we will read the bitmap data pMemStm.reset( new SvMemoryStream); pIStm = pMemStm.get(); - pMemStm->SetBuffer( pData, nUncodedSize, false, nUncodedSize ); + assert(!aData.empty()); + pMemStm->SetBuffer( &aData.front(), nUncodedSize, false, nUncodedSize ); nOffset = 0; } else @@ -892,11 +928,6 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon } } - if( pData ) - { - rtl_freeMemory(pData); - } - Bitmap::ReleaseAccess(pAcc); if(bAlphaPossible) commit 4c241896ffab41da0cc1bcbf7e3401f205da28a1 Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 15:14:08 2016 +0100 absolute seek is clearer Change-Id: Iec8ff121e630bc6f63f935af248edce4dd572428 diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index ede4c55..e2767cc 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -845,8 +845,8 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon aCodec.Read( rIStm, pData, nUncodedSize ); aCodec.EndCompression(); - // skip unread bytes from coded buffer - rIStm.SeekRel( nCodedSize - ( rIStm.Tell() - nCodedPos ) ); + // Seek behind the encoded block. There might have been bytes left or the codec might have read more than necessary. + rIStm.Seek(nCodedSize + nCodedPos); // set decoded bytes to memory stream, // from which we will read the bitmap data commit 55141ac82950aaa289fd5ec9957800030fcdba0c Author: David Tardon <dtar...@redhat.com> Date: Fri Jan 22 15:14:00 2016 +0100 sanitize value Change-Id: I0dfde2343263251a6b3034736c5c7219c5e130e4 diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx index e5b67fa..ede4c55 100644 --- a/vcl/source/gdi/dibtools.cxx +++ b/vcl/source/gdi/dibtools.cxx @@ -835,6 +835,8 @@ bool ImplReadDIBBody( SvStream& rIStm, Bitmap& rBmp, Bitmap* pBmpAlpha, sal_uLon // read coding information rIStm.ReadUInt32( nCodedSize ).ReadUInt32( nUncodedSize ).ReadUInt32( aHeader.nCompression ); + if (nCodedSize > rIStm.remainingSize()) + nCodedSize = sal_uInt32(rIStm.remainingSize()); pData = static_cast<sal_uInt8*>(rtl_allocateMemory( nUncodedSize )); // decode buffer _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits