valgrind: multiple problems in cgm filter

valgrind: Mismatched free() / delete / delete []

(cherry picked from commit 4d82ccb1edcc700f1ff8387ec51ac8114d5d77e3)

valgrind: Mismatched free() / delete / delete []

(cherry picked from commit 61550120b45640898677c7f765a491737445954f)

valgrind: Source and destination overlap in memcpy

(cherry picked from commit b2b5bc10b6574e4717f651602c970bdb20abd1a7)

sal_True->true

(cherry picked from commit aeea1cab4300371797fc1283c1e8623fa8238908)

check for short reads

(cherry picked from commit 2e234939874575a41673b3ceb4a759afa3b333aa)

valgrind: multiple errors

(cherry picked from commit b251762c6549639975a7e9962c9fb5b365ef2063)

stray ;s all other the place

(cherry picked from commit 740617901dc2e7a1effd211c755ea9d00b32094b)

valgrind: init GroupLevels

(cherry picked from commit b13591d48f58b45f4465ce03bd1248ec1e95584f)

Change-Id: Idccebc826ade29a507e115108d20fd461b7bb4ec
Reviewed-on: https://gerrit.libreoffice.org/11977
Reviewed-by: Michael Stahl <mstahl@redhat.com>
Tested-by: Michael Stahl <mstahl@redhat.com>

5 files changed, 60 insertions, 98 deletions

diff --git a/filter/source/graphicfilter/icgm/actimpr.cxx b/filter/source/graphicfilter/icgm/actimpr.cxx
index 8db2cc36cda1..ba96bdea3471 100644
--- a/filter/source/graphicfilter/icgm/actimpr.cxx
+++ b/filter/source/graphicfilter/icgm/actimpr.cxx
@@ -74,9 +74,7 @@ CGMImpressOutAct::CGMImpressOutAct( CGM& rCGM, const uno::Reference< frame::XMod
         }
         mpCGM->mbStatus = bStatRet;
     }
-};
-
-
+}
 
 bool CGMImpressOutAct::ImplInitPage()
 {
@@ -176,9 +174,7 @@ void CGMImpressOutAct::ImplSetLineBundle()
         aAny <<= aLineDash;
         maXPropSet->setPropertyValue( "LineDash", aAny );
     }
-};
-
-
+}
 
 void CGMImpressOutAct::ImplSetFillBundle()
 {
@@ -343,9 +339,7 @@ void CGMImpressOutAct::ImplSetFillBundle()
         aAny <<= aHatch;
         maXPropSet->setPropertyValue( "FillHatch", aAny );
     }
-};
-
-
+}
 
 void CGMImpressOutAct::ImplSetTextBundle( const uno::Reference< beans::XPropertySet > & rProperty )
 {
@@ -387,9 +381,7 @@ void CGMImpressOutAct::ImplSetTextBundle( const uno::Reference< beans::XProperty
     }
     aAny <<= aFontDescriptor;
     rProperty->setPropertyValue( "FontDescriptor", aAny );
-};
-
-
+}
 
 void CGMImpressOutAct::InsertPage()
 {
@@ -401,9 +393,7 @@ void CGMImpressOutAct::InsertPage()
             mpCGM->mbStatus = false;
     }
     mnCurrentPage++;
-};
-
-
+}
 
 void CGMImpressOutAct::BeginGroup()
 {
@@ -413,9 +403,7 @@ void CGMImpressOutAct::BeginGroup()
     }
     mnGroupLevel++;
     mnGroupActCount = mpCGM->mnActCount;
-};
-
-
+}
 
 void CGMImpressOutAct::EndGroup()
 {
@@ -446,9 +434,7 @@ void CGMImpressOutAct::EndGroup()
             }
         }
     }
-};
-
-
+}
 
 void CGMImpressOutAct::EndGrouping()
 {
@@ -458,8 +444,6 @@ void CGMImpressOutAct::EndGrouping()
     }
 }
 
-
-
 void CGMImpressOutAct::DrawRectangle( FloatRect& rFloatRect )
 {
     if ( mnGroupActCount != ( mpCGM->mnActCount - 1 ) )         // POWERPOINT HACK !!!
@@ -472,9 +456,7 @@ void CGMImpressOutAct::DrawRectangle( FloatRect& rFloatRect )
             ImplSetFillBundle();
         }
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawEllipse( FloatPoint& rCenter, FloatPoint& rSize, double& rOrientation )
 {
@@ -499,9 +481,7 @@ void CGMImpressOutAct::DrawEllipse( FloatPoint& rCenter, FloatPoint& rSize, doub
         }
         ImplSetFillBundle();
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawEllipticalArc( FloatPoint& rCenter, FloatPoint& rSize, double& rOrientation,
             sal_uInt32 nType, double& fStartAngle, double& fEndAngle )
@@ -572,9 +552,7 @@ void CGMImpressOutAct::DrawEllipticalArc( FloatPoint& rCenter, FloatPoint& rSize
             }
         }
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawBitmap( CGMBitmapDescriptor* pBmpDesc )
 {
@@ -613,9 +591,7 @@ void CGMImpressOutAct::DrawBitmap( CGMBitmapDescriptor* pBmpDesc )
 
         }
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawPolygon( Polygon& rPoly )
 {
@@ -645,10 +621,7 @@ void CGMImpressOutAct::DrawPolygon( Polygon& rPoly )
         maXPropSet->setPropertyValue( "PolyPolygon", aParam );
         ImplSetFillBundle();
     }
-};
-
-
-
+}
 
 void CGMImpressOutAct::DrawPolyLine( Polygon& rPoly )
 {
@@ -678,9 +651,7 @@ void CGMImpressOutAct::DrawPolyLine( Polygon& rPoly )
         maXPropSet->setPropertyValue( "PolyPolygon", aParam );
         ImplSetLineBundle();
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawPolybezier( Polygon& rPolygon )
 {
@@ -713,9 +684,7 @@ void CGMImpressOutAct::DrawPolybezier( Polygon& rPolygon )
         maXPropSet->setPropertyValue( "PolyPolygonBezier", aParam );
         ImplSetLineBundle();
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawPolyPolygon( PolyPolygon& rPolyPolygon )
 {
@@ -758,9 +727,7 @@ void CGMImpressOutAct::DrawPolyPolygon( PolyPolygon& rPolyPolygon )
         maXPropSet->setPropertyValue( "PolyPolygonBezier", aParam);
         ImplSetFillBundle();
     }
-};
-
-
+}
 
 void CGMImpressOutAct::DrawText( awt::Point& rTextPos, awt::Size& rTextSize, char* pString, sal_uInt32 /*nSize*/, FinalFlag eFlag )
 {
@@ -907,9 +874,7 @@ void CGMImpressOutAct::DrawText( awt::Point& rTextPos, awt::Size& rTextSize, cha
             nFinalTextCount = maXShapes->getCount();
         }
     }
-};
-
-
+}
 
 void CGMImpressOutAct::AppendText( char* pString, sal_uInt32 /*nSize*/, FinalFlag /*eFlag*/ )
 {
diff --git a/filter/source/graphicfilter/icgm/bundles.cxx b/filter/source/graphicfilter/icgm/bundles.cxx
index bfd80459d14e..a4a9100cda92 100644
--- a/filter/source/graphicfilter/icgm/bundles.cxx
+++ b/filter/source/graphicfilter/icgm/bundles.cxx
@@ -87,8 +87,6 @@ FillBundle& FillBundle::operator=( FillBundle& rSource )
     return *this;
 };
 
-
-
 FontEntry::FontEntry() :
     pFontName       ( NULL ),
     eCharSetType    ( CST_CCOMPLETE ),
@@ -99,12 +97,10 @@ FontEntry::FontEntry() :
 
 FontEntry::~FontEntry()
 {
-    delete pFontName;
+    delete [] pFontName;
     delete pCharSetValue;
 }
 
-
-
 CGMFList::CGMFList() :
     nFontNameCount      ( 0 ),
     nCharSetCount       ( 0 ),
@@ -209,7 +205,7 @@ void CGMFList::InsertName( sal_uInt8* pSource, sal_uInt32 nSize )
         sal_uInt32 nToCopy = nSize - nToCopyOfs - nPrev;
         if ( nToCopy )
         {
-            memcpy( pFound, pFound + nToCopyOfs, nToCopy );
+            memmove( pFound, pFound + nToCopyOfs, nToCopy );
         }
         nSize -= nToCopyOfs;
     }
@@ -229,7 +225,7 @@ void CGMFList::InsertName( sal_uInt8* pSource, sal_uInt32 nSize )
         sal_uInt32 nToCopy = nSize - nToCopyOfs - nPrev;
         if ( nToCopy )
         {
-            memcpy( pFound, pFound + nToCopyOfs, nToCopy );
+            memmove( pFound, pFound + nToCopyOfs, nToCopy );
         }
         nSize -= nToCopyOfs;
     }
diff --git a/filter/source/graphicfilter/icgm/cgm.cxx b/filter/source/graphicfilter/icgm/cgm.cxx
index a9808b717d43..d0aa5fb87d90 100644
--- a/filter/source/graphicfilter/icgm/cgm.cxx
+++ b/filter/source/graphicfilter/icgm/cgm.cxx
@@ -57,6 +57,7 @@ CGM::CGM( sal_uInt32 nMode, uno::Reference< frame::XModel > & rModel )
     , mpChart(NULL)
     , mpOutAct(new CGMImpressOutAct(*this, rModel))
     , mpSource(NULL)
+    , mpEndValidSource(NULL)
     , mnParaSize(0)
     , mnActCount(0)
     , mpBuf(NULL)
@@ -74,7 +75,6 @@ CGM::CGM( sal_uInt32 nMode, uno::Reference< frame::XModel > & rModel )
 
 CGM::~CGM()
 {
-
     if ( mpGraphic )
     {
         mpGDIMetaFile->Stop();
@@ -84,7 +84,7 @@ CGM::~CGM()
         *mpGraphic = Graphic( *mpGDIMetaFile );
     }
     for( size_t i = 0, n = maDefRepList.size(); i < n; ++i )
-        delete maDefRepList[ i ];
+        delete [] maDefRepList[i];
     maDefRepList.clear();
     maDefRepSizeList.clear();
     delete mpBitmapInUse;
@@ -103,6 +103,8 @@ sal_uInt32 CGM::GetBackGroundColor()
 sal_uInt32 CGM::ImplGetUI16( sal_uInt32 /*nAlign*/ )
 {
     sal_uInt8* pSource = mpSource + mnParaSize;
+    if (pSource + 2 > mpEndValidSource)
+        throw css::uno::Exception("attempt to read past end of input", 0);
     mnParaSize += 2;
     return ( pSource[ 0 ] << 8 ) +  pSource[ 1 ];
 };
@@ -115,6 +117,8 @@ sal_uInt8 CGM::ImplGetByte( sal_uInt32 nSource, sal_uInt32 nPrecision )
 long CGM::ImplGetI( sal_uInt32 nPrecision )
 {
     sal_uInt8* pSource = mpSource + mnParaSize;
+    if (pSource + nPrecision > mpEndValidSource)
+        throw css::uno::Exception("attempt to read past end of input", 0);
     mnParaSize += nPrecision;
     switch( nPrecision )
     {
@@ -145,6 +149,8 @@ long CGM::ImplGetI( sal_uInt32 nPrecision )
 sal_uInt32 CGM::ImplGetUI( sal_uInt32 nPrecision )
 {
     sal_uInt8* pSource = mpSource + mnParaSize;
+    if (pSource + nPrecision > mpEndValidSource)
+        throw css::uno::Exception("attempt to read past end of input", 0);
     mnParaSize += nPrecision;
     switch( nPrecision )
     {
@@ -194,12 +200,18 @@ double CGM::ImplGetFloat( RealPrecision eRealPrecision, sal_uInt32 nRealSize )
     float   fFloatBuf;
 
 #ifdef OSL_BIGENDIAN
-        bCompatible = sal_True;
+    bCompatible = true;
 #else
-        bCompatible = false;
+    bCompatible = false;
 #endif
+
+    if (mpSource + mnParaSize + nRealSize > mpEndValidSource)
+        throw css::uno::Exception("attempt to read past end of input", 0);
+
     if ( bCompatible )
+    {
         pPtr = mpSource + mnParaSize;
+    }
     else
     {
         if ( nRealSize == 4 )
@@ -620,11 +632,13 @@ void CGM::ImplDefaultReplacement()
         sal_uInt32  nOldElementID = mnElementID;
         sal_uInt32  nOldElementSize = mnElementSize;
         sal_uInt8*  pOldBuf = mpSource;
+        sal_uInt8*  pOldEndValidSource = mpEndValidSource;
 
         for ( size_t i = 0, n = maDefRepList.size(); i < n; ++i )
         {
             sal_uInt8*  pBuf = maDefRepList[ i ];
             sal_uInt32  nElementSize = maDefRepSizeList[ i ];
+            mpEndValidSource = pBuf + nElementSize;
             sal_uInt32  nCount = 0;
             while ( mbStatus && ( nCount < nElementSize ) )
             {
@@ -653,6 +667,7 @@ void CGM::ImplDefaultReplacement()
         mnElementID = nOldElementID;
         mnParaSize = mnElementSize = nOldElementSize;
         mpSource = pOldBuf;
+        mpEndValidSource = pOldEndValidSource;
     }
 }
 
@@ -663,7 +678,9 @@ bool CGM::Write( SvStream& rIStm )
 
     mnParaSize = 0;
     mpSource = mpBuf;
-    rIStm.Read( mpSource, 2 );
+    if (rIStm.Read(mpSource, 2) != 2)
+        return false;
+    mpEndValidSource = mpSource + 2;
     mnEscape = ImplGetUI16();
     mnElementClass = mnEscape >> 12;
     mnElementID = ( mnEscape & 0x0fe0 ) >> 5;
@@ -671,12 +688,18 @@ bool CGM::Write( SvStream& rIStm )
 
     if ( mnElementSize == 31 )
     {
-        rIStm.Read( mpSource + mnParaSize, 2 );
+        if (rIStm.Read(mpSource + mnParaSize, 2) != 2)
+            return false;
+        mpEndValidSource = mpSource + mnParaSize + 2;
         mnElementSize = ImplGetUI16();
     }
     mnParaSize = 0;
-    if ( mnElementSize )
-        rIStm.Read( mpSource + mnParaSize, mnElementSize );
+    if (mnElementSize)
+    {
+        if (rIStm.Read(mpSource, mnElementSize) != mnElementSize)
+            return false;
+        mpEndValidSource = mpSource + mnElementSize;
+    }
 
     if ( mnElementSize & 1 )
         rIStm.SeekRel( 1 );
@@ -744,7 +767,7 @@ ImportCGM( OUString& rFileName, uno::Reference< frame::XModel > & rXModel, sal_u
                 }
             }
         }
-        catch( const ::com::sun::star::uno::Exception& )
+        catch (const css::uno::Exception&)
         {
             nStatus = 0;
         }
diff --git a/filter/source/graphicfilter/icgm/cgm.hxx b/filter/source/graphicfilter/icgm/cgm.hxx
index 25ab80a64970..7148a401ecda 100644
--- a/filter/source/graphicfilter/icgm/cgm.hxx
+++ b/filter/source/graphicfilter/icgm/cgm.hxx
@@ -79,8 +79,9 @@ class CGM
         ::std::vector< sal_uInt8 * > maDefRepList;
         ::std::vector< sal_uInt32  > maDefRepSizeList;
 
-        sal_uInt8*              mpSource;       // source buffer that is not increased
-                                            // ( instead use mnParaCount to index )
+        sal_uInt8*              mpSource;         // start of source buffer that is not increased
+                                                  // ( instead use mnParaCount to index )
+        sal_uInt8*              mpEndValidSource; // end position in source buffer of last valid data
         sal_uInt32              mnParaSize;     // actual parameter size which has been done so far
         sal_uInt32              mnActCount;     // increased by each action
         sal_uInt8*              mpBuf;          // source stream operation -> then this is allocated for
diff --git a/filter/source/graphicfilter/icgm/outact.cxx b/filter/source/graphicfilter/icgm/outact.cxx
index f7394dfdc3f1..521d894569d2 100644
--- a/filter/source/graphicfilter/icgm/outact.cxx
+++ b/filter/source/graphicfilter/icgm/outact.cxx
@@ -20,37 +20,31 @@
 
 #include <outact.hxx>
 #include <vcl/gradient.hxx>
+#include <string.h>
 
 using namespace ::com::sun::star;
 
-
-
 CGMOutAct::CGMOutAct( CGM& rCGM )
 {
     mpCGM = &rCGM;
     mnCurrentPage = 0;
     mnGroupActCount = mnGroupLevel = 0;
-    mpGroupLevel = new sal_uInt32[ CGM_OUTACT_MAX_GROUP_LEVEL ];
+    mpGroupLevel = new sal_uInt32[CGM_OUTACT_MAX_GROUP_LEVEL];
+    memset(mpGroupLevel, 0, sizeof(sal_uInt32) * CGM_OUTACT_MAX_GROUP_LEVEL);
     mpPoints = (Point*)new sal_Int8[ 0x2000 * sizeof( Point ) ];
     mpFlags = new sal_uInt8[ 0x2000 ];
 
     mnIndex = 0;
     mpGradient = NULL;
-};
-
-
+}
 
 CGMOutAct::~CGMOutAct()
 {
     delete[] (sal_Int8*) mpPoints;
     delete[] mpFlags;
     delete[] mpGroupLevel;
-
-    if ( mpGradient )
-        delete mpGradient;
-};
-
-
+    delete mpGradient;
+}
 
 void CGMOutAct::BeginFigure()
 {
@@ -61,8 +55,6 @@ void CGMOutAct::BeginFigure()
     mnIndex = 0;
 }
 
-
-
 void CGMOutAct::CloseRegion()
 {
     if ( mnIndex > 2 )
@@ -73,8 +65,6 @@ void CGMOutAct::CloseRegion()
     }
 }
 
-
-
 void CGMOutAct::NewRegion()
 {
     if ( mnIndex > 2 )
@@ -85,8 +75,6 @@ void CGMOutAct::NewRegion()
     mnIndex = 0;
 }
 
-
-
 void CGMOutAct::EndFigure()
 {
     NewRegion();
@@ -96,8 +84,6 @@ void CGMOutAct::EndFigure()
     mnIndex = 0;
 }
 
-
-
 void CGMOutAct::RegPolyLine( Polygon& rPolygon, bool bReverse )
 {
     sal_uInt16 nPoints = rPolygon.GetSize();
@@ -123,8 +109,6 @@ void CGMOutAct::RegPolyLine( Polygon& rPolygon, bool bReverse )
     }
 }
 
-
-
 void CGMOutAct::SetGradientOffset( long nHorzOfs, long nVertOfs, sal_uInt32 /*nType*/ )
 {
     if ( !mpGradient )
@@ -133,8 +117,6 @@ void CGMOutAct::SetGradientOffset( long nHorzOfs, long nVertOfs, sal_uInt32 /*nT
     mpGradient->YOffset = ( (sal_uInt16)nVertOfs & 0x7f );
 }
 
-
-
 void CGMOutAct::SetGradientAngle( long nAngle )
 {
     if ( !mpGradient )
@@ -142,8 +124,6 @@ void CGMOutAct::SetGradientAngle( long nAngle )
     mpGradient->Angle = sal::static_int_cast< sal_Int16 >(nAngle);
 }
 
-
-
 void CGMOutAct::SetGradientDescriptor( sal_uInt32 nColorFrom, sal_uInt32 nColorTo )
 {
     if ( !mpGradient )
@@ -152,8 +132,6 @@ void CGMOutAct::SetGradientDescriptor( sal_uInt32 nColorFrom, sal_uInt32 nColorT
     mpGradient->EndColor = nColorTo;
 }
 
-
-
 void CGMOutAct::SetGradientStyle( sal_uInt32 nStyle, double /*fRatio*/ )
 {
     if ( !mpGradient )
@@ -187,5 +165,4 @@ void CGMOutAct::SetGradientStyle( sal_uInt32 nStyle, double /*fRatio*/ )
     }
 }
 
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */