package/qa/cppunit/data/tdf163341.ods |binary
package/qa/cppunit/data/tdf163364.ods |binary
package/qa/cppunit/test_zippackage.cxx | 41 +++++++++++++++++++++++++++++++
package/source/zipapi/ZipFile.cxx | 10 +++----
package/source/zippackage/ZipPackage.cxx | 8 ++++++
5 files changed, 54 insertions(+), 5 deletions(-)
New commits:
commit cf87f49f7e9633b6369f2c1c6bb85efb24679212
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Mon Oct 14 13:52:12 2024 +0200
Commit: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
CommitDate: Mon Oct 28 14:06:04 2024 +0100
tdf#163341 package: fix reading Zip64 produced by stream-write-ods
1. Accept 0xFFFF as nEndDisk/nEndDirDisk - the Zip APPNOTE says that
values that don't fit into 16 bits SHOULD be 0xFFFF but it doesn't
prohibit values that do fit (like, uhm, 0) to be written as 0xFFFF
(regression from commit ca21cc985d57fffe7c834159b17c095206304994)
2. Fix misuse of o3tl::make_unsigned - it requires non-negative value,
just do signed compare instead
3. Fix bad conversion from pointer to optional in
ZipFile::readExtraFields() which effectively prevented the offset
from being read
(regression from commit efae4fc42d5fe3c0a69757226f38efc10d101194)
Change-Id: Ib5e7776a30834e507b297fb28266b5489d1ab68d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174898
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit 279f42fa8b19d4fe81c3bba4c7af21aa8ab135b9)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174795
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174911
Reviewed-by: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
Tested-by: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
Reviewed-by: Adolfo Jayme Barrientos <fit...@ubuntu.com>
diff --git a/package/qa/cppunit/data/tdf163341.ods
b/package/qa/cppunit/data/tdf163341.ods
new file mode 100644
index 000000000000..5971e0123883
Binary files /dev/null and b/package/qa/cppunit/data/tdf163341.ods differ
diff --git a/package/qa/cppunit/test_zippackage.cxx
b/package/qa/cppunit/test_zippackage.cxx
index 892f2979cd61..e8f3f910bb5a 100644
--- a/package/qa/cppunit/test_zippackage.cxx
+++ b/package/qa/cppunit/test_zippackage.cxx
@@ -412,6 +412,19 @@ CPPUNIT_TEST_FIXTURE(ZipPackageTest, testTdf163364)
}
}
+CPPUNIT_TEST_FIXTURE(ZipPackageTest, testTdf163341)
+{
+ auto const
url(m_directories.getURLFromSrc(u"/package/qa/cppunit/data/tdf163341.ods"));
+ uno::Sequence<uno::Any> const args{
+ uno::Any(url),
+ uno::Any(beans::NamedValue("StorageFormat",
uno::Any(embed::StorageFormats::PACKAGE)))
+ };
+
+ // this Zip64 should load successfully
+
m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext(ZipPackage,
args,
+
m_xContext);
+}
+
//CPPUNIT_TEST_SUITE_REGISTRATION(...);
//CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/package/source/zipapi/ZipFile.cxx
b/package/source/zipapi/ZipFile.cxx
index 2e82433fb0da..5b5502cb5ea1 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1158,12 +1158,12 @@ std::tuple<sal_Int64, sal_Int64, sal_Int64>
ZipFile::findCentralDirectory()
aGrabber.seek(nEndPos + 4);
sal_uInt16 const nEndDisk = aGrabber.ReadUInt16();
- if (nEndDisk != 0)
+ if (nEndDisk != 0 && nEndDisk != 0xFFFF)
{ // only single disk is supported!
throw ZipException(u"invalid end (disk)"_ustr );
}
sal_uInt16 const nEndDirDisk = aGrabber.ReadUInt16();
- if (nEndDirDisk != 0)
+ if (nEndDirDisk != 0 && nEndDisk != 0xFFFF)
{
throw ZipException(u"invalid end (directory disk)"_ustr );
}
@@ -1245,12 +1245,12 @@ std::tuple<sal_Int64, sal_Int64, sal_Int64>
ZipFile::findCentralDirectory()
{
throw ZipException(u"inconsistent Zip/Zip64 end
(entries)"_ustr);
}
- if (o3tl::make_unsigned(nEndDirSize) != sal_uInt32(-1)
+ if (nEndDirSize != -1
&& nEnd64DirSize != nEndDirSize)
{
throw ZipException(u"inconsistent Zip/Zip64 end
(size)"_ustr);
}
- if (o3tl::make_unsigned(nEndDirOffset) != sal_uInt32(-1)
+ if (nEndDirOffset != -1
&& nEnd64DirOffset != nEndDirOffset)
{
throw ZipException(u"inconsistent Zip/Zip64 end
(offset)"_ustr);
@@ -1525,7 +1525,7 @@ bool ZipFile::readExtraFields(MemoryByteGrabber&
aMemGrabber, sal_Int16 nExtraLe
{
nCompressedSize = aMemGrabber.ReadUInt64();
nReadSize = 16;
- if (dataSize >= 24 && roOffset)
+ if (dataSize >= 24)
{
roOffset.emplace(aMemGrabber.ReadUInt64());
nReadSize = 24;
commit 53369b37eff1a9994af88be35fd297fcfb9a5c27
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Mon Oct 14 12:04:05 2024 +0200
Commit: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
CommitDate: Mon Oct 28 14:05:50 2024 +0100
tdf#163364 package: ask to recover for this invalid ODF package
Bugdoc has a data descriptor on a folder entry, which is very odd and
entirely pointless. Which is also the first entry, so it's an invalid
ODF package anyway.
ZipPackageFolder throws UnknownPropertyException for "WasEncrypted",
which results in General I/O error, but we want to ask the user if the
file should be opened in recovery mode.
(regression from commit 32cad89592ec04ab552399095c91dd76afb3002c)
Change-Id: Iafe610d507cf92d2fd2e9c3040592c3e638a30dd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174889
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit 3efad499bf4f7623610a54f9f14622de4954352f)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174790
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/174910
Reviewed-by: Adolfo Jayme Barrientos <fit...@ubuntu.com>
Tested-by: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
Reviewed-by: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
diff --git a/package/qa/cppunit/data/tdf163364.ods
b/package/qa/cppunit/data/tdf163364.ods
new file mode 100644
index 000000000000..a772aebdbc7e
Binary files /dev/null and b/package/qa/cppunit/data/tdf163364.ods differ
diff --git a/package/qa/cppunit/test_zippackage.cxx
b/package/qa/cppunit/test_zippackage.cxx
index 624457ab7bcf..892f2979cd61 100644
--- a/package/qa/cppunit/test_zippackage.cxx
+++ b/package/qa/cppunit/test_zippackage.cxx
@@ -384,6 +384,34 @@ CPPUNIT_TEST_FIXTURE(ZipPackageTest, testZip64End)
}
}
+CPPUNIT_TEST_FIXTURE(ZipPackageTest, testTdf163364)
+{
+ auto const
url(m_directories.getURLFromSrc(u"/package/qa/cppunit/data/tdf163364.ods"));
+ uno::Sequence<uno::Any> const args{
+ uno::Any(url),
+ uno::Any(beans::NamedValue("StorageFormat",
uno::Any(embed::StorageFormats::PACKAGE)))
+ };
+
+ // don't load corrupted zip file
+
CPPUNIT_ASSERT_THROW(m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext(
+ ZipPackage, args, m_xContext),
+ css::packages::zip::ZipIOException);
+
+ try
+ {
+ uno::Sequence<uno::Any> const args2{
+ uno::Any(url), uno::Any(beans::NamedValue(u"RepairPackage"_ustr,
uno::Any(true))),
+ uno::Any(beans::NamedValue("StorageFormat",
uno::Any(embed::StorageFormats::ZIP)))
+ };
+
m_xContext->getServiceManager()->createInstanceWithArgumentsAndContext(ZipPackage,
args2,
+
m_xContext);
+ }
+ catch (css::packages::zip::ZipIOException const&)
+ {
+ // check that this doesn't crash, it doesn't matter if it succeeds or
not
+ }
+}
+
//CPPUNIT_TEST_SUITE_REGISTRATION(...);
//CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/package/source/zippackage/ZipPackage.cxx
b/package/source/zippackage/ZipPackage.cxx
index 116cf8dcb1a7..1d5ddca152ec 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -192,6 +192,14 @@ void ZipPackage::checkZipEntriesWithDD()
{
uno::Reference<XPropertySet> xStream;
getByHierarchicalName(rEntry.sPath) >>= xStream;
+ uno::Reference<XServiceInfo> const xStreamSI{xStream,
uno::UNO_QUERY_THROW};
+ if
(!xStreamSI->supportsService("com.sun.star.packages.PackageStream"))
+ {
+ SAL_INFO("package", "entry STORED with data descriptor is
folder: \"" << rEntry.sPath << "\"");
+ throw ZipIOException(
+ THROW_WHERE
+ "entry STORED with data descriptor is folder");
+ }
if (!xStream->getPropertyValue("WasEncrypted").get<bool>())
{
SAL_INFO("package", "entry STORED with data descriptor but
not encrypted: \"" << rEntry.sPath << "\"");
