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

[Caolán McNamara]

 vcl/source/gdi/dibtools.cxx |   49 +++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 16 deletions(-)

New commits:
commit c66cd027864a7cab9cb2dc722439c848b769451c
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Tue Sep 19 11:43:16 2017 +0100

    ofz#2869 avoid oom with bmp rle images
    
    and to the sanity checks on remaining data size *after*
    the seek to the offset, which requires moving the read
    of the palette to remain before that seek
    
    Change-Id: I687a79fb3f109556c1a7aaa9423f77a1eb98a3cf
    Reviewed-on: https://gerrit.libreoffice.org/42461
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/vcl/source/gdi/dibtools.cxx b/vcl/source/gdi/dibtools.cxx
index f2dc2c25f474..82cac86ae0f4 100644
--- a/vcl/source/gdi/dibtools.cxx
+++ b/vcl/source/gdi/dibtools.cxx
@@ -872,6 +872,22 @@ bool ImplReadDIBBody(SvStream& rIStm, Bitmap& rBmp, AlphaMask* pBmpAlpha, sal_uL
         pIStm = &rIStm;
     }
 
+    // read palette
+    BitmapPalette aPalette;
+    if (nColors)
+    {
+        aPalette.SetEntryCount(nColors);
+        ImplReadDIBPalette(*pIStm, aPalette, aHeader.nSize != DIBCOREHEADERSIZE);
+    }
+
+    if (pIStm->GetError())
+        return false;
+
+    if (nOffset)
+    {
+        pIStm->SeekRel(nOffset - (pIStm->Tell() - nStmPos));
+    }
+
     const sal_Int64 nBitsPerLine (static_cast<sal_Int64>(aHeader.nWidth) * static_cast<sal_Int64>(aHeader.nBitCount));
     if (nBitsPerLine > SAL_MAX_UINT32)
         return false;
@@ -880,13 +896,30 @@ bool ImplReadDIBBody(SvStream& rIStm, Bitmap& rBmp, AlphaMask* pBmpAlpha, sal_uL
     switch (aHeader.nCompression)
     {
         case RLE_8:
+        {
             if (aHeader.nBitCount != 8)
                 return false;
+            // (partially) check the image dimensions to avoid potential large bitmap allocation if the input is damaged
+            sal_uInt64 nMaxWidth = pIStm->remainingSize();
+            nMaxWidth *= 256;   //assume generous compression ratio
+            if (aHeader.nHeight != 0)
+                nMaxWidth /= aHeader.nHeight;
+            if (nMaxWidth < static_cast<sal_uInt64>(aHeader.nWidth))
+                return false;
             break;
+        }
         case RLE_4:
+        {
             if (aHeader.nBitCount != 4)
                 return false;
+            sal_uInt64 nMaxWidth = pIStm->remainingSize();
+            nMaxWidth *= 512;   //assume generous compression ratio
+            if (aHeader.nHeight != 0)
+                nMaxWidth /= aHeader.nHeight;
+            if (nMaxWidth < static_cast<sal_uInt64>(aHeader.nWidth))
+                return false;
             break;
+        }
         case BITFIELDS:
             break;
         case ZCOMPRESS:
@@ -931,17 +964,6 @@ bool ImplReadDIBBody(SvStream& rIStm, Bitmap& rBmp, AlphaMask* pBmpAlpha, sal_uL
         pAccAlpha = AlphaMask::ScopedWriteAccess(aNewBmpAlpha);
     }
 
-    // read palette
-    BitmapPalette aPalette;
-    if (nColors)
-    {
-        aPalette.SetEntryCount(nColors);
-        ImplReadDIBPalette(*pIStm, aPalette, aHeader.nSize != DIBCOREHEADERSIZE);
-    }
-
-    if (pIStm->GetError())
-        return false;
-
     sal_uInt16 nBitCount(discretizeBitcount(aHeader.nBitCount));
     const BitmapPalette* pPal = &aPalette;
     //ofz#948 match the surrounding logic of case TransparentType::Bitmap of
@@ -964,11 +986,6 @@ bool ImplReadDIBBody(SvStream& rIStm, Bitmap& rBmp, AlphaMask* pBmpAlpha, sal_uL
         return false;
     }
 
-    if(nOffset)
-    {
-        pIStm->SeekRel(nOffset - (pIStm->Tell() - nStmPos));
-    }
-
     // read bits
     bool bAlphaUsed(false);
     bool bRet = ImplReadDIBBits(*pIStm, aHeader, *pAcc, aPalette, pAccAlpha.get(), bTopDown, bAlphaUsed, nAlignedWidth, bForceToMonoWhileReading);