package/source/zipapi/ZipFile.cxx | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
New commits:
commit e7c30e6c1ee8fbea7d44ab00e2721ff13dfa4da5
Author: Caolán McNamara <caol...@redhat.com>
AuthorDate: Tue Apr 25 12:58:29 2023 +0100
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Tue Apr 25 15:10:35 2023 +0200
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 <caol...@redhat.com>
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 );
+ }
}
}
}
