Resolves: tdf#155005 use of uninitialised value

==2515797== Conditional jump or move depends on uninitialised value(s)
==2515797==    at 0x33FAB399: ZipFile::recover() (ZipFile.cxx:1090)
==2515797==    by 0x33FA4D32: ZipFile::ZipFile(rtl::Reference<comphelper::RefCountedMutex>, com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext>, bool, bool) (ZipFile.cxx:111)
==2515797==    by 0x33FEF134: void std::_Construct<ZipFile, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(ZipFile*, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (stl_construct.h:119)
==2515797==    by 0x33FED528: void std::_Optional_payload_base<ZipFile>::_M_construct<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:278)
==2515797==    by 0x33FEB73B: void std::_Optional_base_impl<ZipFile, std::_Optional_base<ZipFile, false, false> >::_M_construct<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:457)
==2515797==    by 0x33FE77DB: std::enable_if<is_constructible_v<ZipFile, rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>, ZipFile&>::type std::optional<ZipFile>::emplace<rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, bool&>(rtl::Reference<comphelper::RefCountedMutex>&, com::sun::star::uno::Reference<com::sun::star::io::XInputStream>&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool&&, bool&) (optional:918)
==2515797==    by 0x33FD61FD: ZipPackage::initialize(com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (ZipPackage.cxx:760)
==2515797==    by 0x64DE1EC: cppuhelper::ServiceManager::Data::Implementation::doCreateInstanceWithArguments(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (servicemanager.cxx:732)
==2515797==    by 0x64DDF19: cppuhelper::ServiceManager::Data::Implementation::createInstanceWithArguments(com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&, bool, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (servicemanager.cxx:694)
==2515797==    by 0x64E0E8F: cppuhelper::ServiceManager::createInstanceWithArgumentsAndContext(rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&, com::sun::star::uno::Reference<com::sun::star::uno::XComponentContext> const&) (servicemanager.cxx:1018)
==2515797==    by 0x34A3B25F: OStorage_Impl::OpenOwnPackage() (xstorage.cxx:435)
==2515797==    by 0x34A3C16C: OStorage_Impl::ReadContents() (xstorage.cxx:541)
==2515797==  Uninitialised value was created by a stack allocation
==2515797==    at 0x33FAB02C: ZipFile::recover() (ZipFile.cxx:1034)

since:

commit abda72eeac19b18c22f57d5443c3955a463605d7
Date:   Mon Feb 20 00:32:22 2023 +0100

tdf#82984 tdf#94915 zip64 support (import + export)

where

- aEntry.nCompressedSize = nCompressedSize;
- aEntry.nSize = nSize;

was removed before the subsequent use of aEntry.nCompressedSize/aEntry.nSize

change things (git show -w for clarity) to check bounds first just for
the range the extra fields might be in and read those, and then follow
up with the original check that the newly discovered
aEntry.nCompressedSize/aEntry.nSize are within the bounds of the overall
file

Change-Id: Iad4ce8297109b06bc5baf77df4f3e86659cbb4cf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150969
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

1 files changed, 18 insertions, 13 deletions

diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index c1260a7fbf2d..59bdcf8de891 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1084,10 +1084,9 @@ void ZipFile::recover()
                             sal_Int32 nDescrLength =
                                 ( aEntry.nMethod == DEFLATED && ( aEntry.nFlag & 8 ) ) ? 16 : 0;
 
-                            sal_Int64 nDataSize = ( aEntry.nMethod == DEFLATED ) ? aEntry.nCompressedSize : aEntry.nSize;
-                            sal_Int64 nBlockLength = nDataSize + aEntry.nPathLen + aEntry.nExtraLen + 30 + nDescrLength;
+                            sal_Int64 nBlockHeaderLength = aEntry.nPathLen + aEntry.nExtraLen + 30 + nDescrLength;
                             if ( aEntry.nPathLen >= 0 && aEntry.nExtraLen >= 0
-                                && ( nGenPos + nPos + nBlockLength ) <= nLength )
+                                && ( nGenPos + nPos + nBlockHeaderLength ) <= nLength )
                             {
                                 // read always in UTF8, some tools seem not to set UTF8 bit
                                 if( nPos + 30 + aEntry.nPathLen <= nBufSize )
@@ -1128,19 +1127,25 @@ void ZipFile::recover()
                                     }
                                 }
 
-                                aEntry.nCompressedSize = nCompressedSize;
-                                aEntry.nSize = nSize;
+                                sal_Int64 nDataSize = ( aEntry.nMethod == DEFLATED ) ? nCompressedSize : nSize;
+                                sal_Int64 nBlockLength = nDataSize + nBlockHeaderLength;
 
-                                aEntry.nOffset = nGenPos + nPos + 30 + aEntry.nPathLen + aEntry.nExtraLen;
-
-                                if ( ( aEntry.nSize || aEntry.nCompressedSize ) && !checkSizeAndCRC( aEntry ) )
+                                if (( nGenPos + nPos + nBlockLength ) <= nLength )
                                 {
-                                    aEntry.nCrc = 0;
-                                    aEntry.nCompressedSize = 0;
-                                    aEntry.nSize = 0;
-                                }
+                                    aEntry.nCompressedSize = nCompressedSize;
+                                    aEntry.nSize = nSize;
 
-                                aEntries.emplace( aEntry.sPath, aEntry );
+                                    aEntry.nOffset = nGenPos + nPos + 30 + aEntry.nPathLen + aEntry.nExtraLen;
+
+                                    if ( ( aEntry.nSize || aEntry.nCompressedSize ) && !checkSizeAndCRC( aEntry ) )
+                                    {
+                                        aEntry.nCrc = 0;
+                                        aEntry.nCompressedSize = 0;
+                                        aEntry.nSize = 0;
+                                    }
+
+                                    aEntries.emplace( aEntry.sPath, aEntry );
+                                }
                             }
                         }
                     }