ofz: check reads, zero vars, return earlier on failure

Change-Id: I287ec575d95ff950097a255abbcd0d3304c04a56

1 files changed, 90 insertions, 58 deletions

diff --git a/vcl/source/gdi/svmconverter.cxx b/vcl/source/gdi/svmconverter.cxx
index b1e3be8957e9..099c7917ae4b 100644
--- a/vcl/source/gdi/svmconverter.cxx
+++ b/vcl/source/gdi/svmconverter.cxx
@@ -53,29 +53,47 @@ void ImplWriteRect( SvStream& rOStm, const Rectangle& rRect )
     WritePair( rOStm, rRect.BottomRight() );
 }
 
-void ImplReadPoly( SvStream& rIStm, tools::Polygon& rPoly )
+bool ImplReadPoly(SvStream& rIStm, tools::Polygon& rPoly)
 {
-    sal_Int32   nSize;
+    sal_Int32 nSize32(0);
+    rIStm.ReadInt32(nSize32);
+    sal_uInt16 nSize = nSize32;
 
-    rIStm.ReadInt32( nSize );
-    rPoly = tools::Polygon( (sal_uInt16) nSize );
+    const size_t nMaxPossiblePoints = rIStm.remainingSize() / 2 * sizeof(sal_Int32);
+    if (nSize > nMaxPossiblePoints)
+    {
+        SAL_WARN("vcl.gdi", "svm record claims to have: " << nSize << " points, but only " << nMaxPossiblePoints << " possible");
+        return false;
+    }
+
+    rPoly = tools::Polygon(nSize);
 
-    for( sal_uInt16 i = 0; i < (sal_uInt16) nSize; i++ )
-        ReadPair( rIStm, rPoly[ i ] );
+    for (sal_uInt16 i = 0; i < nSize && rIStm.good(); ++i)
+        ReadPair(rIStm, rPoly[i]);
+
+    return rIStm.good();
 }
 
-void ImplReadPolyPoly( SvStream& rIStm, tools::PolyPolygon& rPolyPoly )
+bool ImplReadPolyPoly(SvStream& rIStm, tools::PolyPolygon& rPolyPoly)
 {
-    tools::Polygon aPoly;
-    sal_Int32   nPolyCount;
+    bool bSuccess = true;
 
-    rIStm.ReadInt32( nPolyCount );
+    tools::Polygon aPoly;
+    sal_Int32 nPolyCount32(0);
+    rIStm.ReadInt32(nPolyCount32);
+    sal_uInt16 nPolyCount = (sal_uInt16)nPolyCount32;
 
-    for( sal_uInt16 i = 0; i < (sal_uInt16) nPolyCount; i++ )
+    for (sal_uInt16 i = 0; i < nPolyCount && rIStm.good(); ++i)
     {
-        ImplReadPoly( rIStm, aPoly );
-        rPolyPoly.Insert( aPoly );
+        if (!ImplReadPoly(rIStm, aPoly))
+        {
+            bSuccess = false;
+            break;
+        }
+        rPolyPoly.Insert(aPoly);
     }
+
+    return bSuccess && rIStm.good();
 }
 
 void ImplWritePolyPolyAction( SvStream& rOStm, const tools::PolyPolygon& rPolyPoly )
@@ -109,7 +127,7 @@ void ImplWritePolyPolyAction( SvStream& rOStm, const tools::PolyPolygon& rPolyPo
 
 void ImplReadColor( SvStream& rIStm, Color& rColor )
 {
-    sal_Int16 nVal;
+    sal_Int16 nVal(0);
 
     rIStm.ReadInt16( nVal ); rColor.SetRed( sal::static_int_cast<sal_uInt8>((sal_uInt16)nVal >> 8) );
     rIStm.ReadInt16( nVal ); rColor.SetGreen( sal::static_int_cast<sal_uInt8>((sal_uInt16)nVal >> 8) );
@@ -775,32 +793,35 @@ void SVMConverter::ImplConvertFromSVM1( SvStream& rIStm, GDIMetaFile& rMtf )
 
             case GDI_POLYLINE_ACTION:
             {
-                ImplReadPoly( rIStm, aActionPoly );
-                nLastPolygonAction = rMtf.GetActionSize();
+                if (ImplReadPoly(rIStm, aActionPoly))
+                {
+                    nLastPolygonAction = rMtf.GetActionSize();
 
-                if( bFatLine )
-                    rMtf.AddAction( new MetaPolyLineAction( aActionPoly, aLineInfo ) );
-                else
-                    rMtf.AddAction( new MetaPolyLineAction( aActionPoly ) );
+                    if( bFatLine )
+                        rMtf.AddAction( new MetaPolyLineAction( aActionPoly, aLineInfo ) );
+                    else
+                        rMtf.AddAction( new MetaPolyLineAction( aActionPoly ) );
+                }
             }
             break;
 
             case GDI_POLYGON_ACTION:
             {
-                ImplReadPoly( rIStm, aActionPoly );
-
-                if( bFatLine )
-                {
-                    rMtf.AddAction( new MetaPushAction( PushFlags::LINECOLOR ) );
-                    rMtf.AddAction( new MetaLineColorAction( COL_TRANSPARENT, false ) );
-                    rMtf.AddAction( new MetaPolygonAction( aActionPoly ) );
-                    rMtf.AddAction( new MetaPopAction() );
-                    rMtf.AddAction( new MetaPolyLineAction( aActionPoly, aLineInfo ) );
-                }
-                else
+                if (ImplReadPoly(rIStm, aActionPoly))
                 {
-                    nLastPolygonAction = rMtf.GetActionSize();
-                    rMtf.AddAction( new MetaPolygonAction( aActionPoly ) );
+                    if( bFatLine )
+                    {
+                        rMtf.AddAction( new MetaPushAction( PushFlags::LINECOLOR ) );
+                        rMtf.AddAction( new MetaLineColorAction( COL_TRANSPARENT, false ) );
+                        rMtf.AddAction( new MetaPolygonAction( aActionPoly ) );
+                        rMtf.AddAction( new MetaPopAction() );
+                        rMtf.AddAction( new MetaPolyLineAction( aActionPoly, aLineInfo ) );
+                    }
+                    else
+                    {
+                        nLastPolygonAction = rMtf.GetActionSize();
+                        rMtf.AddAction( new MetaPolygonAction( aActionPoly ) );
+                    }
                 }
             }
             break;
@@ -809,22 +830,23 @@ void SVMConverter::ImplConvertFromSVM1( SvStream& rIStm, GDIMetaFile& rMtf )
             {
                 tools::PolyPolygon aPolyPoly;
 
-                ImplReadPolyPoly( rIStm, aPolyPoly );
-
-                if( bFatLine )
+                if (ImplReadPolyPoly(rIStm, aPolyPoly))
                 {
-                    rMtf.AddAction( new MetaPushAction( PushFlags::LINECOLOR ) );
-                    rMtf.AddAction( new MetaLineColorAction( COL_TRANSPARENT, false ) );
-                    rMtf.AddAction( new MetaPolyPolygonAction( aPolyPoly ) );
-                    rMtf.AddAction( new MetaPopAction() );
+                    if( bFatLine )
+                    {
+                        rMtf.AddAction( new MetaPushAction( PushFlags::LINECOLOR ) );
+                        rMtf.AddAction( new MetaLineColorAction( COL_TRANSPARENT, false ) );
+                        rMtf.AddAction( new MetaPolyPolygonAction( aPolyPoly ) );
+                        rMtf.AddAction( new MetaPopAction() );
 
-                    for( sal_uInt16 nPoly = 0, nCount = aPolyPoly.Count(); nPoly < nCount; nPoly++ )
-                        rMtf.AddAction( new MetaPolyLineAction( aPolyPoly[ nPoly ], aLineInfo ) );
-                }
-                else
-                {
-                    nLastPolygonAction = rMtf.GetActionSize();
-                    rMtf.AddAction( new MetaPolyPolygonAction( aPolyPoly ) );
+                        for( sal_uInt16 nPoly = 0, nCount = aPolyPoly.Count(); nPoly < nCount; nPoly++ )
+                            rMtf.AddAction( new MetaPolyLineAction( aPolyPoly[ nPoly ], aLineInfo ) );
+                    }
+                    else
+                    {
+                        nLastPolygonAction = rMtf.GetActionSize();
+                        rMtf.AddAction( new MetaPolyPolygonAction( aPolyPoly ) );
+                    }
                 }
             }
             break;
@@ -1088,27 +1110,37 @@ void SVMConverter::ImplConvertFromSVM1( SvStream& rIStm, GDIMetaFile& rMtf )
 
                     case 2:
                     {
-                        ImplReadPoly( rIStm, aActionPoly );
-                        aRegion = vcl::Region( aActionPoly );
-                        bClip = true;
+                        if (ImplReadPoly(rIStm, aActionPoly))
+                        {
+                            aRegion = vcl::Region( aActionPoly );
+                            bClip = true;
+                        }
                     }
                     break;
 
                     case 3:
                     {
+                        bool bSuccess = true;
                         tools::PolyPolygon aPolyPoly;
-                        sal_Int32       nPolyCount;
-
-                        rIStm.ReadInt32( nPolyCount );
+                        sal_Int32 nPolyCount32(0);
+                        rIStm.ReadInt32(nPolyCount32);
+                        sal_uInt16 nPolyCount(nPolyCount32);
 
-                        for( sal_uInt16 j = 0; j < (sal_uInt16) nPolyCount; j++ )
+                        for (sal_uInt16 j = 0; j < nPolyCount && rIStm.good(); ++j)
                         {
-                            ImplReadPoly( rIStm, aActionPoly );
-                            aPolyPoly.Insert( aActionPoly );
+                            if (!ImplReadPoly(rIStm, aActionPoly))
+                            {
+                                bSuccess = false;
+                                break;
+                            }
+                            aPolyPoly.Insert(aActionPoly);
                         }
 
-                        aRegion = vcl::Region( aPolyPoly );
-                        bClip = true;
+                        if (bSuccess)
+                        {
+                            aRegion = vcl::Region( aPolyPoly );
+                            bClip = true;
+                        }
                     }
                     break;
                 }