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>
diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index c1260a7f..59bdcf8 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 );
}
}
}
}
