[Libreoffice-commits] core.git: 7 commits - vcl/source

David Tardon dtardon at redhat.com
Sun Jan 24 07:56:52 PST 2016


 vcl/source/gdi/dibtools.cxx |  175 +++++++++++++++++++++++++-------------------
 1 file changed, 103 insertions(+), 72 deletions(-)

New commits:
commit fdcc9e0b8f083d9964d195ebcddcdeedd40b871d
Author: David Tardon <dtardon at 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 <dtardon at 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 <dtardon at 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 <dtardon at 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 <dtardon at 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 <dtardon at 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 <dtardon at 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


More information about the Libreoffice-commits mailing list