[Libreoffice-commits] core.git: sot/source

[Caolán McNamara]

 sot/source/sdstor/stgcache.cxx |   35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

New commits:
commit f1c790ca3613a43dac84e2a9b6a99d1338176325
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Sat Nov 4 16:53:20 2017 +0000

    ofz short read considered as a successful full block read
    
    i.e StgDataStrm::Read takes the bool of no error and
    multiplies it by the block size to report the length
    read. A short read isn't an error so full buffer is
    considered valid.
    
    To keep #i73846# working and get deterministic fuzzing
    results, zero out the trailing space of a successful but
    short read. Changing this to return the truthful
    number of bytes read makes #i73846# stop working.
    
    There's wonderful nonsense here calculating nPg2,
    determining nBytes to read derived from this,
    reading nBytes into the buffer and then considering
    it an error if nPg2 is not 1 after the presumably
    potentially fatal buffer overflow if nPg2 wasn't initially
    1, but this doesn't seem possible in practice
    
    Change-Id: I2bac6066deb8a2902677e04696367ba2c351b325
    Reviewed-on: https://gerrit.libreoffice.org/44310
    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/sot/source/sdstor/stgcache.cxx b/sot/source/sdstor/stgcache.cxx
index 8387f0290765..8f77abdbc345 100644
--- a/sot/source/sdstor/stgcache.cxx
+++ b/sot/source/sdstor/stgcache.cxx
@@ -319,6 +319,7 @@ void StgCache::Close()
 
 bool StgCache::Read( sal_Int32 nPage, void* pBuf )
 {
+    sal_uInt32 nRead = 0, nBytes = m_nPageSize;
     if( Good() )
     {
         /*  #i73846# real life: a storage may refer to a page one-behind the
@@ -329,28 +330,40 @@ bool StgCache::Read( sal_Int32 nPage, void* pBuf )
             SetError( SVSTREAM_READ_ERROR );
         else if ( nPage < m_nPages )
         {
-            sal_uInt32 nPos = Page2Pos( nPage );
-            sal_Int32 nPg2 = ( ( nPage + 1 ) > m_nPages ) ? m_nPages - nPage : 1;
-            sal_uInt32 nBytes = nPg2 * m_nPageSize;
+            sal_uInt32 nPos;
+            sal_Int32 nPg2;
             // fixed address and size for the header
             if( nPage == -1 )
             {
                 nPos = 0;
-                nBytes = 512;
                 nPg2 = 1;
+                nBytes = 512;
             }
-            if( m_pStrm->Tell() != nPos )
+            else
             {
-                m_pStrm->Seek(nPos);
+                nPos = Page2Pos(nPage);
+                nPg2 = ((nPage + 1) > m_nPages) ? m_nPages - nPage : 1;
             }
-            m_pStrm->ReadBytes( pBuf, nBytes );
-            if ( 1 != nPg2 )
-                SetError( SVSTREAM_READ_ERROR );
+
+            if (m_pStrm->Tell() != nPos)
+                m_pStrm->Seek(nPos);
+
+            if (nPg2 != 1)
+                SetError(SVSTREAM_READ_ERROR);
             else
-                SetError( m_pStrm->GetError() );
+            {
+                nRead = m_pStrm->ReadBytes(pBuf, nBytes);
+                SetError(m_pStrm->GetError());
+            }
         }
     }
-    return Good();
+
+    if (!Good())
+        return false;
+
+    if (nRead != nBytes)
+        memset(static_cast<sal_uInt8*>(pBuf) + nRead, 0, nBytes - nRead);
+    return true;
 }
 
 bool StgCache::Write( sal_Int32 nPage, void const * pBuf )