[Libreoffice-commits] core.git: filter/source svl/source

[Caolán McNamara (via logerrit)]

 filter/source/msfilter/mstoolbar.cxx |    5 ++--
 filter/source/msfilter/svdfppt.cxx   |   39 +++++++++++++++++++----------------
 svl/source/items/macitem.cxx         |   15 +++++++------
 3 files changed, 33 insertions(+), 26 deletions(-)

New commits:
commit f0c2e0d27ccdeaefb00b63e7462e1c25e18f73af
Author:     Caolán McNamara <caolanm at redhat.com>
AuthorDate: Wed Jan 29 17:17:24 2020 +0000
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Wed Jan 29 20:24:02 2020 +0100

    cid#1458020 Untrusted loop bound
    
    cid#1458018 Untrusted loop bound
    cid#1242844 Untrusted loop bound
    
    Change-Id: I9062240290708f4b51b0ce42a30897b50d1a2677
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/87702
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/filter/source/msfilter/mstoolbar.cxx b/filter/source/msfilter/mstoolbar.cxx
index f44cd2bbb3ca..e32181019602 100644
--- a/filter/source/msfilter/mstoolbar.cxx
+++ b/filter/source/msfilter/mstoolbar.cxx
@@ -676,10 +676,11 @@ bool TBCCDData::Read( SvStream &rS)
     rS.ReadInt16( cwstrItems );
     if (cwstrItems > 0)
     {
+        auto nItems = o3tl::make_unsigned(cwstrItems);
         //each WString is at least one byte
-        if (rS.remainingSize() < o3tl::make_unsigned(cwstrItems))
+        if (rS.remainingSize() < nItems)
             return false;
-        for( sal_Int32 index=0; index < cwstrItems; ++index )
+        for (decltype(nItems) index = 0; index < nItems; ++index)
         {
             WString aString;
             if ( !aString.Read( rS ) )
diff --git a/filter/source/msfilter/svdfppt.cxx b/filter/source/msfilter/svdfppt.cxx
index 1cae88f4870f..a1c57622f865 100644
--- a/filter/source/msfilter/svdfppt.cxx
+++ b/filter/source/msfilter/svdfppt.cxx
@@ -1213,24 +1213,29 @@ SdrObject* SdrEscherImport::ProcessObj( SvStream& rSt, DffObjData& rObjData, Svx
             {
                 if ( aSecPropSet.SeekToContent( DFF_Prop_tableRowProperties, rSt ) )
                 {
-                    sal_Int16 i, nRowCount = 0;
-                    rSt.ReadInt16( nRowCount ).ReadInt16( i ).ReadInt16( i );
-                    const size_t nMinRecordSize = 4;
-                    const size_t nMaxRecords = rSt.remainingSize() / nMinRecordSize;
-                    if (nRowCount > 0 && o3tl::make_unsigned(nRowCount) > nMaxRecords)
+                    sal_Int16 i, nReadRowCount = 0;
+                    rSt.ReadInt16( nReadRowCount ).ReadInt16( i ).ReadInt16( i );
+                    if (nReadRowCount > 0)
                     {
-                        SAL_WARN("filter.ms", "Parsing error: " << nMaxRecords <<
-                                 " max possible entries, but " << nRowCount << " claimed, truncating");
-                        nRowCount = nMaxRecords;
-                    }
-                    if (nRowCount > 0)
-                    {
-                        std::unique_ptr<sal_uInt32[]> pTableArry(new sal_uInt32[ nRowCount + 2 ]);
-                        pTableArry[ 0 ] = nTableProperties;
-                        pTableArry[ 1 ] = nRowCount;
-                        for ( i = 0; i < nRowCount; i++ )
-                            rSt.ReadUInt32( pTableArry[ i + 2 ] );
-                        rData.pTableRowProperties = std::move(pTableArry);
+                        const size_t nMinRecordSize = 4;
+                        const size_t nMaxRecords = rSt.remainingSize() / nMinRecordSize;
+
+                        auto nRowCount = o3tl::make_unsigned(nReadRowCount);
+                        if (nRowCount > nMaxRecords)
+                        {
+                            SAL_WARN("filter.ms", "Parsing error: " << nMaxRecords <<
+                                     " max possible entries, but " << nRowCount << " claimed, truncating");
+                            nRowCount = nMaxRecords;
+                        }
+                        if (nRowCount > 0)
+                        {
+                            std::unique_ptr<sal_uInt32[]> pTableArry(new sal_uInt32[ nRowCount + 2 ]);
+                            pTableArry[ 0 ] = nTableProperties;
+                            pTableArry[ 1 ] = nRowCount;
+                            for (decltype(nRowCount) nRow = 0; nRow < nRowCount; ++nRow)
+                                rSt.ReadUInt32(pTableArry[nRow + 2]);
+                            rData.pTableRowProperties = std::move(pTableArry);
+                        }
                     }
                 }
             }
diff --git a/svl/source/items/macitem.cxx b/svl/source/items/macitem.cxx
index 64a22aa0039c..b0750212e3dc 100644
--- a/svl/source/items/macitem.cxx
+++ b/svl/source/items/macitem.cxx
@@ -86,28 +86,30 @@ void SvxMacroTableDtor::Read( SvStream& rStrm )
     sal_uInt16 nVersion;
     rStrm.ReadUInt16( nVersion );
 
-    short nMacro(0);
-    rStrm.ReadInt16(nMacro);
-    if (nMacro < 0)
+    short nReadMacro(0);
+    rStrm.ReadInt16(nReadMacro);
+    if (nReadMacro < 0)
     {
-        SAL_WARN("editeng", "Parsing error: negative value " << nMacro);
+        SAL_WARN("editeng", "Parsing error: negative value " << nReadMacro);
         return;
     }
 
+    auto nMacro = o3tl::make_unsigned(nReadMacro);
+
     const size_t nMinStringSize = rStrm.GetStreamCharSet() == RTL_TEXTENCODING_UNICODE ? 4 : 2;
     size_t nMinRecordSize = 2 + 2*nMinStringSize;
     if( SVX_MACROTBL_VERSION40 <= nVersion )
         nMinRecordSize+=2;
 
     const size_t nMaxRecords = rStrm.remainingSize() / nMinRecordSize;
-    if (o3tl::make_unsigned(nMacro) > nMaxRecords)
+    if (nMacro > nMaxRecords)
     {
         SAL_WARN("editeng", "Parsing error: " << nMaxRecords <<
                  " max possible entries, but " << nMacro<< " claimed, truncating");
         nMacro = nMaxRecords;
     }
 
-    for (short i = 0; i < nMacro; ++i)
+    for (decltype(nMacro) i = 0; i < nMacro; ++i)
     {
         sal_uInt16 nCurKey, eType = STARBASIC;
         OUString aLibName, aMacName;
@@ -122,7 +124,6 @@ void SvxMacroTableDtor::Read( SvStream& rStrm )
     }
 }
 
-
 SvStream& SvxMacroTableDtor::Write( SvStream& rStream ) const
 {
     sal_uInt16 nVersion = SOFFICE_FILEFORMAT_31 == rStream.GetVersion()