Steps to reproduce: 1. Open attachment 155137 [details] from bug 128244 2. Click on 'Yes' when the repair dialog is prompted -> Crash Reproduced in Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community Build ID: a1388417b217430de456868c440817459697c4a8 CPU threads: 8; OS: Linux 5.10; UI render: default; VCL: gtk3 Locale: es-ES (es_ES.UTF-8); UI: en-US Calc: threaded
Regression introduced by: author Caolán McNamara <caolanm@redhat.com> 2023-03-09 14:32:06 +0000 committer Caolán McNamara <caolanm@redhat.com> 2023-03-09 20:17:15 +0000 commit b6d1cb887438733da2465f107da5088f9826435e (patch) tree f8b4aeed4c1cd8ce27055630efd3b02f1861c790 parent d898620cb47e0d794d4033fdfa13b049f972a9ff (diff) cid#1521901 Pointer to local outside scope Bisected with: bibisect-linux64-7.6 Adding Cc: to Caolán McNamara
Created attachment 186911 [details] bt with debug symbols On pc Debian x86-64 with master sources updated today, I got an assertion.
I don't get a crash but I do get a valgrind warning of: ==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)
I think the problem is earlier with commit abda72eeac19b18c22f57d5443c3955a463605d7 Date: Mon Feb 20 00:32:22 2023 +0100 tdf#82984 tdf#94915 zip64 support (import + export)
nCompressedSize and nSize are defined as sal_uInt64 After line 1081 of package/source/zipapi/ZipFile.cxx nCompressedSize=4294967295 nSize=4294967295 so a size which is ok for sal_Int64 After readExtraFields (line 1126) nCompressedSize=18446744073709551615 nSize=18446744073709551615 so a size which need sal_uInt64 the pb is "aEntry" is a ZipEntry struct with: sal_Int64 nCompressedSize; sal_Int64 nSize; so the big size is transformed into -1
yeah, https://gerrit.libreoffice.org/c/core/+/150968 addresses that. It might be that we should be using sal_uInt64 throughout, but seeing as we are using sal_Int64 it looks safest to leave all that alone and just call the resulting -1 invalid. I see another problem, the uninitialized value, which for some will lead into this -1 case and for others not by pure change and propose https://gerrit.libreoffice.org/c/core/+/150969 for that
Caolán McNamara committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/80805716a409c34203b059f3e03cd934367186c3 tdf#155005 fail gracefully on encountering a negative compression value It will be available in 7.6.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Caolán McNamara committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/e7c30e6c1ee8fbea7d44ab00e2721ff13dfa4da5 Resolves: tdf#155005 use of uninitialised value It will be available in 7.6.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Caolán McNamara committed a patch related to this issue. It has been pushed to "libreoffice-7-5": https://git.libreoffice.org/core/commit/e4d598e350d39c7354b1855fb55108413f0f7114 tdf#155005 fail gracefully on encountering a negative compression value It will be available in 7.5.4. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Caolán McNamara committed a patch related to this issue. It has been pushed to "libreoffice-7-5-3": https://git.libreoffice.org/core/commit/4173fa8150e5b31a6f40ba119d3b4dcdf3340695 tdf#155005 fail gracefully on encountering a negative compression value It will be available in 7.5.3. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I don't get a crash now, so I'll mark as fixed. Assuming Caolán just forgot to do so. Interesting side note: if I try to do a save-as, I get an error.
(In reply to Justin L from comment #11) > Interesting side note: if I try to do a save-as, I get an error. Ignore: the example comes from bug 128244 which also has a "normal" export32.ods which exhibits the same save-as error. So completely unrelated to zip64.