[Libreoffice-commits] core.git: 4 commits - sc/inc sc/source

[Caolán McNamara]

 sc/inc/address.hxx                  |    8 ++++----
 sc/source/filter/excel/excform.cxx  |   10 ++++++++++
 sc/source/filter/excel/xihelper.cxx |   10 ++++++++++
 sc/source/filter/excel/xilink.cxx   |   11 +++++++++++
 4 files changed, 35 insertions(+), 4 deletions(-)

New commits:
commit 8921054fe8b819ef52d0e0b6aee84314677e90f2
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Mon Nov 17 20:40:12 2014 +0000

    Related: coverity#1242793 Untrusted value as argument
    
    why doesn't coverity consider that Valid[Tab|Row|Col] check the lower bound of
    nPos.
    
    Could it need to be as simple as naively looking for a ">="
    
    Change-Id: Id80f9d30b9166caef20b74569f7b50a569189d71

diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx
index 7fcdee1..07ebe02 100644
--- a/sc/inc/address.hxx
+++ b/sc/inc/address.hxx
@@ -110,22 +110,22 @@ const SCROW W16MAXROW = W16MAXROWCOUNT - 1;
 //  old stuff defines end
 inline bool ValidCol( SCCOL nCol )
 {
-    return static_cast<SCCOL>(0) <= nCol && nCol <= MAXCOL;
+    return nCol >= static_cast<SCCOL>(0) && nCol <= MAXCOL;
 }
 
 inline bool ValidRow( SCROW nRow )
 {
-    return static_cast<SCROW>(0) <= nRow && nRow <= MAXROW;
+    return nRow >= static_cast<SCROW>(0) && nRow <= MAXROW;
 }
 
 inline bool ValidTab( SCTAB nTab )
 {
-    return static_cast<SCTAB>(0) <= nTab && nTab <= MAXTAB;
+    return nTab >= static_cast<SCTAB>(0) && nTab <= MAXTAB;
 }
 
 inline bool ValidTab( SCTAB nTab, SCTAB nMaxTab )
 {
-    return static_cast<SCTAB>(0) <= nTab && nTab <= nMaxTab;
+    return nTab >= static_cast<SCTAB>(0) && nTab <= nMaxTab;
 }
 
 inline bool ValidColRow( SCCOL nCol, SCROW nRow )
commit 206d68d587ee106c1a51db8507268fdf21fa1ddc
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Mon Nov 17 20:34:55 2014 +0000

    coverity#1242538 Untrusted loop bound
    
    Change-Id: I663f70d6324c6dd42208aa3804edfe3680881ea1

diff --git a/sc/source/filter/excel/excform.cxx b/sc/source/filter/excel/excform.cxx
index eda4134..630997a 100644
--- a/sc/source/filter/excel/excform.cxx
+++ b/sc/source/filter/excel/excform.cxx
@@ -1791,6 +1791,16 @@ void ExcelToSc::ReadExtensionArray( unsigned int n, XclImpStream& aIn )
         OSL_FAIL( "ExcelToSc::ReadExtensionArray - missing matrix" );
     }
 
+    //assuming worse case scenario of unknown types
+    const size_t nMinRecordSize = 1;
+    const size_t nMaxRows = aIn.GetRecLeft() / (nMinRecordSize * nCols);
+    if (nRows > nMaxRows)
+    {
+        SAL_WARN("sc", "Parsing error: " << nMaxRows <<
+                 " max possible rows, but " << nRows << " claimed, truncating");
+        nRows = nMaxRows;
+    }
+
     svl::SharedStringPool& rPool = GetDoc().GetSharedStringPool();
     for( nR = 0 ; nR < nRows; nR++ )
     {
commit 07ec99d307925b33d13b40d9a0a44ef029025ecc
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Mon Nov 17 20:32:31 2014 +0000

    coverity#1242628 Untrusted loop bound
    
    Change-Id: Ifabdfab76279e4417642ce10cb86a43184b94629

diff --git a/sc/source/filter/excel/xihelper.cxx b/sc/source/filter/excel/xihelper.cxx
index 6a98771..37e8d0e 100644
--- a/sc/source/filter/excel/xihelper.cxx
+++ b/sc/source/filter/excel/xihelper.cxx
@@ -838,6 +838,16 @@ XclImpCachedMatrix::XclImpCachedMatrix( XclImpStream& rStrm ) :
         ++mnScRows;
     }
 
+    //assuming worse case scenario of unknown types
+    const size_t nMinRecordSize = 1;
+    const size_t nMaxRows = rStrm.GetRecLeft() / (nMinRecordSize * mnScCols);
+    if (mnScRows > nMaxRows)
+    {
+        SAL_WARN("sc", "Parsing error: " << nMaxRows <<
+                 " max possible rows, but " << mnScRows << " claimed, truncating");
+        mnScRows = nMaxRows;
+    }
+
     for( SCSIZE nScRow = 0; nScRow < mnScRows; ++nScRow )
         for( SCSIZE nScCol = 0; nScCol < mnScCols; ++nScCol )
             maValueList.push_back( new XclImpCachedValue( rStrm ) );
commit 5aa174b08489f1f217546966d2396bdf56842dca
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Mon Nov 17 20:28:55 2014 +0000

    coverity#1242631 Untrusted loop bound
    
    Change-Id: Ib034582fa4fa6e8149db2e45491230b4900c4d08

diff --git a/sc/source/filter/excel/xilink.cxx b/sc/source/filter/excel/xilink.cxx
index 75bf324..52e8a5a 100644
--- a/sc/source/filter/excel/xilink.cxx
+++ b/sc/source/filter/excel/xilink.cxx
@@ -634,6 +634,17 @@ XclImpSupbook::XclImpSupbook( XclImpStream& rStrm ) :
     else if( nSBTabCnt )
     {
         meType = EXC_SBTYPE_EXTERN;
+
+        //assuming all empty strings with just len header of 0
+        const size_t nMinRecordSize = sizeof(sal_Int16);
+        const size_t nMaxRecords = rStrm.GetRecLeft() / nMinRecordSize;
+        if (nSBTabCnt > nMaxRecords)
+        {
+            SAL_WARN("sc", "Parsing error: " << nMaxRecords <<
+                     " max possible entries, but " << nSBTabCnt << " claimed, truncating");
+            nSBTabCnt = nMaxRecords;
+        }
+
         for( sal_uInt16 nSBTab = 0; nSBTab < nSBTabCnt; ++nSBTab )
         {
             OUString aTabName( rStrm.ReadUniString() );