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@libreoffice.org>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

1 files changed, 24 insertions, 11 deletions

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 )