[Libreoffice-commits] core.git: Branch 'libreoffice-6-0' - sc/source

Eike Rathke erack at redhat.com
Fri Feb 2 21:50:06 UTC 2018


 sc/source/filter/excel/tokstack.cxx |   79 +++++++++++++++++++-----------------
 sc/source/filter/inc/tokstack.hxx   |   24 ++++++++++
 sc/source/filter/lotus/lotform.cxx  |   27 +++++++++---
 3 files changed, 85 insertions(+), 45 deletions(-)

New commits:
commit 6e2f927704980c0057d5deb3a6bc3ef5540379a4
Author: Eike Rathke <erack at redhat.com>
Date:   Fri Dec 1 10:54:01 2017 +0100

    ofz: guard against binary crap argument counts and ID/OpCode generation
    
     This is a combination of 2 commits.
    
    ofz: guard against binary crap argument counts and ID/OpCode generation
    
    (cherry picked from commit 889c72a7e54f241342f42b1b0a05858902228cbc)
    
    Do not force non-existent parameters into the TokenPool, ofz-related
    
    (cherry picked from commit e6ced1496da9580cf885cce1a2fc9f67528c3a0e)
    
    2fa0ae81b987af592c14486040077c9ff157fab9
    
    Change-Id: I60e181729713f3b202e880707a79e9da80d9d85d
    Reviewed-on: https://gerrit.libreoffice.org/49161
    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/sc/source/filter/excel/tokstack.cxx b/sc/source/filter/excel/tokstack.cxx
index 39a33dddd4d5..4129b50f2287 100644
--- a/sc/source/filter/excel/tokstack.cxx
+++ b/sc/source/filter/excel/tokstack.cxx
@@ -108,6 +108,21 @@ bool TokenPool::GrowId()
     return true;
 }
 
+bool TokenPool::CheckElementOrGrow()
+{
+    // Last possible ID to be assigned somewhere is nElementAkt+1
+    if (nElementAkt + 1 == nScTokenOff - 1)
+    {
+        SAL_WARN("sc.filter","TokenPool::CheckElementOrGrow - last possible ID " << nElementAkt+1);
+        return false;
+    }
+
+    if (nElementAkt >= nElement)
+        return GrowElement();
+
+    return true;
+}
+
 bool TokenPool::GrowElement()
 {
     sal_uInt16 nElementNew = lcl_canGrow( nElement);
@@ -161,9 +176,11 @@ bool TokenPool::GrowMatrix()
 
 bool TokenPool::GetElement( const sal_uInt16 nId )
 {
-    OSL_ENSURE( nId < nElementAkt, "*TokenPool::GetElement(): Id too large!?" );
     if (nId >= nElementAkt)
+    {
+        SAL_WARN("sc.filter","TokenPool::GetElement - Id too large, " << nId << " >= " << nElementAkt);
         return false;
+    }
 
     bool bRet = true;
     if( pType[ nId ] == T_Id )
@@ -394,9 +411,8 @@ void TokenPool::operator >>( TokenId& rId )
 {
     rId = static_cast<TokenId>( nElementAkt + 1 );
 
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return;
+    if (!CheckElementOrGrow())
+        return;
 
     pElement[ nElementAkt ] = nP_IdLast;    // Start of Token-sequence
     pType[ nElementAkt ] = T_Id;            // set Typeinfo
@@ -409,9 +425,8 @@ void TokenPool::operator >>( TokenId& rId )
 
 const TokenId TokenPool::Store( const double& rDouble )
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( pP_Dbl.m_writemark >= pP_Dbl.m_capacity )
         if (!pP_Dbl.Grow())
@@ -438,9 +453,8 @@ const TokenId TokenPool::Store( const sal_uInt16 nIndex )
 const TokenId TokenPool::Store( const OUString& rString )
 {
     // mostly copied to Store( const sal_Char* ), to avoid a temporary string
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( ppP_Str.m_writemark >= ppP_Str.m_capacity )
         if (!ppP_Str.Grow())
@@ -468,9 +482,8 @@ const TokenId TokenPool::Store( const OUString& rString )
 
 const TokenId TokenPool::Store( const ScSingleRefData& rTr )
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( ppP_RefTr.m_writemark >= ppP_RefTr.m_capacity )
         if (!ppP_RefTr.Grow())
@@ -492,9 +505,8 @@ const TokenId TokenPool::Store( const ScSingleRefData& rTr )
 
 const TokenId TokenPool::Store( const ScComplexRefData& rTr )
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( ppP_RefTr.m_writemark + 1 >= ppP_RefTr.m_capacity )
         if (!ppP_RefTr.Grow(2))
@@ -522,9 +534,8 @@ const TokenId TokenPool::Store( const ScComplexRefData& rTr )
 
 const TokenId TokenPool::Store( const DefTokenId e, const OUString& r )
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( ppP_Ext.m_writemark >= ppP_Ext.m_capacity )
         if (!ppP_Ext.Grow())
@@ -549,9 +560,8 @@ const TokenId TokenPool::Store( const DefTokenId e, const OUString& r )
 
 const TokenId TokenPool::StoreNlf( const ScSingleRefData& rTr )
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( ppP_Nlf.m_writemark >= ppP_Nlf.m_capacity )
         if (!ppP_Nlf.Grow())
@@ -575,9 +585,8 @@ const TokenId TokenPool::StoreNlf( const ScSingleRefData& rTr )
 
 const TokenId TokenPool::StoreMatrix()
 {
-    if( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     if( nP_MatrixAkt >= nP_Matrix )
         if (!GrowMatrix())
@@ -598,9 +607,8 @@ const TokenId TokenPool::StoreMatrix()
 
 const TokenId TokenPool::StoreName( sal_uInt16 nIndex, sal_Int16 nSheet )
 {
-    if ( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     pElement[nElementAkt] = static_cast<sal_uInt16>(maRangeNames.size());
     pType[nElementAkt] = T_RN;
@@ -617,9 +625,8 @@ const TokenId TokenPool::StoreName( sal_uInt16 nIndex, sal_Int16 nSheet )
 
 const TokenId TokenPool::StoreExtName( sal_uInt16 nFileId, const OUString& rName )
 {
-    if ( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     pElement[nElementAkt] = static_cast<sal_uInt16>(maExtNames.size());
     pType[nElementAkt] = T_ExtName;
@@ -636,9 +643,8 @@ const TokenId TokenPool::StoreExtName( sal_uInt16 nFileId, const OUString& rName
 
 const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabName, const ScSingleRefData& rRef )
 {
-    if ( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     pElement[nElementAkt] = static_cast<sal_uInt16>(maExtCellRefs.size());
     pType[nElementAkt] = T_ExtRefC;
@@ -656,9 +662,8 @@ const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabNa
 
 const TokenId TokenPool::StoreExtRef( sal_uInt16 nFileId, const OUString& rTabName, const ScComplexRefData& rRef )
 {
-    if ( nElementAkt >= nElement )
-        if (!GrowElement())
-            return static_cast<const TokenId>(nElementAkt+1);
+    if (!CheckElementOrGrow())
+        return static_cast<const TokenId>(nElementAkt+1);
 
     pElement[nElementAkt] = static_cast<sal_uInt16>(maExtAreaRefs.size());
     pType[nElementAkt] = T_ExtRefA;
diff --git a/sc/source/filter/inc/tokstack.hxx b/sc/source/filter/inc/tokstack.hxx
index 1e7e0eeb1582..eef80f072c26 100644
--- a/sc/source/filter/inc/tokstack.hxx
+++ b/sc/source/filter/inc/tokstack.hxx
@@ -217,6 +217,13 @@ private:
         bool                        GrowId();
         bool                        GrowElement();
         bool                        GrowMatrix();
+                                    /** @return false means nElementAkt range
+                                        below nScTokenOff would overflow or
+                                        further allocation is not possible, no
+                                        new ID available other than
+                                        nElementAkt+1.
+                                     */
+        bool                        CheckElementOrGrow();
         bool                        GetElement( const sal_uInt16 nId );
         bool                        GetElementRek( const sal_uInt16 nId );
         void                        ClearMatrix();
@@ -317,6 +324,7 @@ inline void TokenStack::operator >>( TokenId& rId )
     else
     {
         SAL_WARN("sc.filter", "*TokenStack::>>(): is empty, is empty, ...");
+        rId = 0;
     }
 }
 
@@ -331,7 +339,13 @@ inline TokenPool& TokenPool::operator <<( const TokenId& rId )
     //       finalize with >> or Store()
     // rId -> ( sal_uInt16 ) rId - 1;
     sal_uInt16 nId = static_cast<sal_uInt16>(rId);
-    if (nId >= nScTokenOff)
+    if (nId == 0)
+    {
+        // This would result in nId-1==0xffff, create error.
+        SAL_WARN("sc.filter", "-TokenPool::operator <<: TokenId 0");
+        nId = static_cast<sal_uInt16>(ocErrNull) + nScTokenOff + 1;
+    }
+    else if (nId >= nScTokenOff)
     {
         SAL_WARN("sc.filter", "-TokenPool::operator <<: TokenId in DefToken-Range! " << static_cast<sal_uInt16>(rId));
 
@@ -374,7 +388,13 @@ inline TokenPool& TokenPool::operator <<( TokenStack& rStack )
         if (!GrowId())
             return *this;
 
-    pP_Id[ nP_IdAkt ] = ( ( sal_uInt16 ) rStack.Get() ) - 1;
+    sal_uInt16 nId = static_cast<sal_uInt16>(rStack.Get());
+    if (nId == 0)
+    {
+        // Indicates error, so generate one. Empty stack, overflow, ...
+        nId = static_cast<sal_uInt16>(ocErrNull) + nScTokenOff + 1;
+    }
+    pP_Id[ nP_IdAkt ] = nId - 1;
     nP_IdAkt++;
 
     return *this;
diff --git a/sc/source/filter/lotus/lotform.cxx b/sc/source/filter/lotus/lotform.cxx
index 068a77f5bb46..091da56e9dbb 100644
--- a/sc/source/filter/lotus/lotform.cxx
+++ b/sc/source/filter/lotus/lotform.cxx
@@ -77,9 +77,13 @@ void LotusToSc::DoFunc( DefTokenId eOc, sal_uInt8 nCnt, const sal_Char* pExtStri
         }
     }
 
-    for( nLauf = 0 ; nLauf < nCnt ; nLauf++ )
+    for( nLauf = 0 ; nLauf < nCnt && aStack.HasMoreTokens() ; nLauf++ )
         aStack >> eParam[ nLauf ];
 
+    if (nLauf < nCnt)
+        // Adapt count to reality. All sort of binary crap is possible.
+        nCnt = static_cast<sal_uInt8>(nLauf);
+
     // special cases...
     switch( eOc )
     {
@@ -189,12 +193,23 @@ void LotusToSc::DoFunc( DefTokenId eOc, sal_uInt8 nCnt, const sal_Char* pExtStri
         sal_Int16 nLast = nCnt - 1;
 
         if( eOc == ocPMT )
-        {   // special case ocPMT, ignore (negate?) last parameter!
+        {   // special case ocPMT, negate last parameter!
             // additionally: 1. -> 3., 3. -> 2., 2. -> 1.
-            SAL_WARN_IF( nCnt != 3, "sc",
-                "+LotusToSc::DoFunc(): ocPMT needs 3 parameters!" );
-            aPool << eParam[ 1 ] << ocSep << eParam[ 0 ] << ocSep
-                << ocNegSub << eParam[ 2 ];
+            SAL_WARN_IF( nCnt != 3, "sc", "+LotusToSc::DoFunc(): ocPMT needs 3 parameters!" );
+            // There should be at least 3 arguments, but with binary crap may not..
+            switch (nCnt)
+            {
+                case 1:
+                    aPool << eParam[ 1 ];
+                break;
+                case 2:
+                    aPool << eParam[ 1 ] << ocSep << eParam[ 0 ];
+                break;
+                default:
+                case 3:
+                    aPool << eParam[ 1 ] << ocSep << eParam[ 0 ] << ocSep << ocNegSub << eParam[ 2 ];
+                break;
+            }
         }
         else
         {   // default


More information about the Libreoffice-commits mailing list