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

[Eike Rathke]

 sc/source/filter/inc/formel.hxx   |   30 +++++++++++++++++++++++++-----
 sc/source/filter/inc/tokstack.hxx |   10 ++++++++--
 2 files changed, 33 insertions(+), 7 deletions(-)

New commits:
commit 047cc1d976516c527f4c7051a98aade59c7494d9
Author: Eike Rathke <erack at redhat.com>
Date:   Fri Nov 10 10:58:44 2017 +0100

    Do not create arbitrary OpCode values from binary garbage, ofz-related
    
    Change-Id: Ifb6f22472a9e9c0be95131bf8f49985ccc17c483

diff --git a/sc/source/filter/inc/tokstack.hxx b/sc/source/filter/inc/tokstack.hxx
index 759e83e88f62..1e7e0eeb1582 100644
--- a/sc/source/filter/inc/tokstack.hxx
+++ b/sc/source/filter/inc/tokstack.hxx
@@ -330,16 +330,22 @@ inline TokenPool& TokenPool::operator <<( const TokenId& rId )
     // POST: rId's are stored consecutively in Pool under a new Id;
     //       finalize with >> or Store()
     // rId -> ( sal_uInt16 ) rId - 1;
-    if ((sal_uInt16)rId >= nScTokenOff)
+    sal_uInt16 nId = static_cast<sal_uInt16>(rId);
+    if (nId >= nScTokenOff)
     {
         SAL_WARN("sc.filter", "-TokenPool::operator <<: TokenId in DefToken-Range! " << static_cast<sal_uInt16>(rId));
+
+        // Do not "invent" OpCode values by arbitrarily mapping into the Calc
+        // space. This badly smells like an overflow or binary garbage, so
+        // treat as error.
+        nId = static_cast<sal_uInt16>(ocErrNull) + nScTokenOff + 1;
     }
 
     if( nP_IdAkt >= nP_Id )
         if (!GrowId())
             return *this;
 
-    pP_Id[ nP_IdAkt ] = ( ( sal_uInt16 ) rId ) - 1;
+    pP_Id[ nP_IdAkt ] = nId - 1;
     nP_IdAkt++;
 
     return *this;
commit 78bcc5ddca186f0009124a697184f332405d3e1e
Author: Eike Rathke <erack at redhat.com>
Date:   Fri Nov 10 10:52:19 2017 +0100

    ofz#4123 do not read past end of file
    
    Change-Id: I1fa3543d541ea084a43a1a11f62680fa798f5647

diff --git a/sc/source/filter/inc/formel.hxx b/sc/source/filter/inc/formel.hxx
index 433ba0809a94..aa7944161439 100644
--- a/sc/source/filter/inc/formel.hxx
+++ b/sc/source/filter/inc/formel.hxx
@@ -143,31 +143,51 @@ inline void LotusConverterBase::Ignore( const long nSeekRel )
 inline void LotusConverterBase::Read( sal_uInt8& nByte )
 {
     aIn.ReadUChar( nByte );
-    nBytesLeft--;
+    if (aIn.good())
+        nBytesLeft--;
+    else
+    {
+        // SvStream::ReadUChar() does not init a single char on failure. This
+        // behaviour is even tested in a unit test.
+        nByte = 0;
+        nBytesLeft = -1;    // bail out early
+    }
 }
 
 inline void LotusConverterBase::Read( sal_uInt16& nUINT16 )
 {
     aIn.ReadUInt16( nUINT16 );
-    nBytesLeft -= 2;
+    if (aIn.good())
+        nBytesLeft -= 2;
+    else
+        nBytesLeft = -1;    // bail out early
 }
 
 inline void LotusConverterBase::Read( sal_Int16& nINT16 )
 {
     aIn.ReadInt16( nINT16 );
-    nBytesLeft -= 2;
+    if (aIn.good())
+        nBytesLeft -= 2;
+    else
+        nBytesLeft = -1;    // bail out early
 }
 
 inline void LotusConverterBase::Read( double& fDouble )
 {
     aIn.ReadDouble( fDouble );
-    nBytesLeft -= 8;
+    if (aIn.good())
+        nBytesLeft -= 8;
+    else
+        nBytesLeft = -1;    // bail out early
 }
 
 inline void LotusConverterBase::Read( sal_uInt32& nUINT32 )
 {
     aIn.ReadUInt32( nUINT32 );
-    nBytesLeft -= 4;
+    if (aIn.good())
+        nBytesLeft -= 4;
+    else
+        nBytesLeft = -1;    // bail out early
 }
 
 #endif