package/inc/ZipPackageFolder.hxx | 2 - package/source/manifest/ManifestExport.cxx | 33 ++++++++++++++++++++++--- package/source/zipapi/ZipFile.cxx | 3 +- package/source/zippackage/ZipPackage.cxx | 26 +++++++++++++++---- package/source/zippackage/ZipPackageFolder.cxx | 13 ++++++++- svx/source/xml/xmlxtexp.cxx | 6 ++++ 6 files changed, 70 insertions(+), 13 deletions(-)
New commits: commit 79d9396911afb5edbaeddf2a18759d100e4e825a Author: Michael Stahl <[email protected]> AuthorDate: Wed Dec 13 21:57:56 2023 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Fri Dec 15 10:49:43 2023 +0100 tdf#105844 package: check for unexpected zip entries on loading ... ... ODF wholesome encrypted package. There can only be "mimetype", "encrypted-package", and files in "META-INF". Change-Id: I5eb46ba29a1a62e25af09e189e0a075a871c71c4 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160718 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit fb9c58a2f32c352e44ffa30e721ef796dc591d33) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160794 diff --git a/package/inc/ZipPackageFolder.hxx b/package/inc/ZipPackageFolder.hxx index cfdcd99d1694..edc46e9c386b 100644 --- a/package/inc/ZipPackageFolder.hxx +++ b/package/inc/ZipPackageFolder.hxx @@ -79,7 +79,7 @@ public: const OUString& GetVersion() const { return m_sVersion; } void SetVersion( const OUString& aVersion ) { m_sVersion = aVersion; } - bool LookForUnexpectedODF12Streams( std::u16string_view aPath ); + bool LookForUnexpectedODF12Streams(std::u16string_view aPath, bool isWholesomeEncryption); void setChildStreamsTypeByExtension( const css::beans::StringPair& aPair ); diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index 459b998de79d..8fb6c27876ab 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -452,7 +452,8 @@ void ZipPackage::parseManifest() m_xRootFolder->removeByName( sMimetype ); } - m_bInconsistent = m_xRootFolder->LookForUnexpectedODF12Streams( std::u16string_view() ); + m_bInconsistent = m_xRootFolder->LookForUnexpectedODF12Streams( + std::u16string_view(), m_xRootFolder->hasByName("encrypted-package")); bool bODF12AndNewer = ( m_xRootFolder->GetVersion().compareTo( ODFVER_012_TEXT ) >= 0 ); if ( !m_bForceRecovery && bODF12AndNewer ) diff --git a/package/source/zippackage/ZipPackageFolder.cxx b/package/source/zippackage/ZipPackageFolder.cxx index 906f36111497..21c71b14cf09 100644 --- a/package/source/zippackage/ZipPackageFolder.cxx +++ b/package/source/zippackage/ZipPackageFolder.cxx @@ -70,7 +70,8 @@ ZipPackageFolder::~ZipPackageFolder() { } -bool ZipPackageFolder::LookForUnexpectedODF12Streams( std::u16string_view aPath ) +bool ZipPackageFolder::LookForUnexpectedODF12Streams( + std::u16string_view const aPath, bool const isWholesomeEncryption) { bool bHasUnexpected = false; @@ -83,10 +84,14 @@ bool ZipPackageFolder::LookForUnexpectedODF12Streams( std::u16string_view aPath // META-INF is not allowed to contain subfolders bHasUnexpected = true; } + else if (isWholesomeEncryption && rShortName != u"META-INF") + { + bHasUnexpected = true; + } else { OUString sOwnPath = aPath + rShortName + "/"; - bHasUnexpected = rInfo.pFolder->LookForUnexpectedODF12Streams( sOwnPath ); + bHasUnexpected = rInfo.pFolder->LookForUnexpectedODF12Streams(sOwnPath, isWholesomeEncryption); } } else @@ -102,6 +107,10 @@ bool ZipPackageFolder::LookForUnexpectedODF12Streams( std::u16string_view aPath // streams from META-INF with expected names are allowed not to be registered in manifest.xml } + else if (isWholesomeEncryption && rShortName != "mimetype" && rShortName != "encrypted-package") + { + bHasUnexpected = true; + } else if ( !rInfo.pStream->IsFromManifest() ) { // the stream is not in META-INF and is not registered in manifest.xml, commit c9f68922a4dd058da5fe885ff030b02f876e8b79 Author: Michael Stahl <[email protected]> AuthorDate: Wed Dec 13 21:53:55 2023 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Fri Dec 15 10:49:28 2023 +0100 tdf#105844 package: remove root document from manifest ... ... for ODF wholesome encryption. 4.3 <manifest:file-entry> For directories, the manifest file should contain a <manifest:file-entry> element only if a directory contains a document or a sub document. Because the "encrypted-package" is not a document but a package, we should probably omit the file-entry for the root document. ZipPackage::writeTempFile() always generates the root document becuase it's needed for GPG properties, and ManifestExport filters it out. A bit tricky to implement, because there isn't a clean distinction between the package and the root document/storage in the package module, in particular there's no other place than the root storage to store the MediaType property. Change-Id: Id7e72a64e2faa074dce80cd5fefb2fa189e2e3ee Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160717 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 91f35f22f0447769c08ca89e27a39b40df18fffa) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160792 diff --git a/package/source/manifest/ManifestExport.cxx b/package/source/manifest/ManifestExport.cxx index 15290ba4f311..a15ae0118277 100644 --- a/package/source/manifest/ManifestExport.cxx +++ b/package/source/manifest/ManifestExport.cxx @@ -81,6 +81,7 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con // find the mediatype of the document if any OUString aDocMediaType; OUString aDocVersion; + bool isWholesomeEncryption(false); const uno::Sequence<beans::PropertyValue>* pRootFolderPropSeq = nullptr; for (const uno::Sequence < beans::PropertyValue >& rSequence : rManList) { @@ -109,12 +110,27 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con if ( aPath == "/" ) { + assert(aDocMediaType.isEmpty()); + // unfortunately no aMediaType in some cases where non-documents + // are stored as StorageFormats::PACKAGE instead of sensible + // StorageFormats::ZIP, such as SvxXMLXTableExportComponent and + // SwXMLTextBlocks, which results in an empty "mimetype" etc but + // can't be easily fixed; try to exclude these cases by checking + // for aVersion, but of course then forgetting to set both version + // and type on an actual document can't be found :( + assert(!aMediaType.isEmpty() || aVersion.isEmpty()); aDocMediaType = aMediaType; aDocVersion = aVersion; pRootFolderPropSeq = &rSequence; - break; + } + + if (aPath == "encrypted-package") + { + isWholesomeEncryption = true; + assert(aDocMediaType.isEmpty() || aDocMediaType == aMediaType); } } + assert(pRootFolderPropSeq); bool bProvideDTD = false; bool bAcceptNonemptyVersion = false; @@ -293,7 +309,12 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con // now write individual file entries for (const uno::Sequence<beans::PropertyValue>& rSequence : rManList) { + if (&rSequence == pRootFolderPropSeq && isWholesomeEncryption) + { + continue; // no root document, but embedded package => omit + } rtl::Reference<::comphelper::AttributeList> pAttrList = new ::comphelper::AttributeList; + OUString fullPath; OUString aString; const uno::Any *pVector = nullptr, *pSalt = nullptr, *pIterationCount = nullptr, *pDigest = nullptr, *pDigestAlg = nullptr, *pEncryptAlg = nullptr, *pStartKeyAlg = nullptr, *pDerivedKeySize = nullptr; for (const beans::PropertyValue& rValue : rSequence) @@ -312,8 +333,8 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con } else if (rValue.Name == sFullPathProperty ) { - rValue.Value >>= aString; - pAttrList->AddAttribute ( ATTRIBUTE_FULL_PATH, aString ); + rValue.Value >>= fullPath; + pAttrList->AddAttribute(ATTRIBUTE_FULL_PATH, fullPath); } else if (rValue.Name == sSizeProperty ) { @@ -338,6 +359,11 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con else if (rValue.Name == sDerivedKeySizeProperty ) pDerivedKeySize = &rValue.Value; } + assert(!fullPath.isEmpty()); + if (isWholesomeEncryption) + { // there may be signatures in META-INF too + assert(fullPath == "encrypted-package" || fullPath.startsWith("META-INF/")); + } xHandler->ignorableWhitespace ( sWhiteSpace ); xHandler->startElement( ELEMENT_FILE_ENTRY , pAttrList); @@ -390,6 +416,7 @@ ManifestExport::ManifestExport( uno::Reference< xml::sax::XDocumentHandler > con } else if (nEncAlgID == xml::crypto::CipherID::AES_GCM_W3C) { + assert(bStoreStartKeyGeneration || pKeyInfoProperty); SAL_WARN_IF(nDerivedKeySize != 32, "package.manifest", "Unexpected key size is provided!"); if (nDerivedKeySize != 32) { diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 664cfa7f1e17..bdcd8610be60 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -647,7 +647,8 @@ uno::Reference< XInputStream > ZipFile::createStreamForZipEntry( #ifndef EMSCRIPTEN static const sal_Int32 nThreadingThreshold = 10000; - if( xSrcStream->available() > nThreadingThreshold ) + // "encrypted-package" is the only data stream, no point in threading it + if (rEntry.sPath != "encrypted-package" && nThreadingThreshold < xSrcStream->available()) xBufStream = new XBufferedThreadedStream(xSrcStream, xSrcStream->getSize()); else #endif diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index 884cfa18937e..459b998de79d 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -417,25 +417,36 @@ void ZipPackage::parseManifest() } } - if ( !bManifestParsed ) + if (!bManifestParsed || m_xRootFolder->GetMediaType().isEmpty()) { // the manifest.xml could not be successfully parsed, this is an inconsistent package if ( aPackageMediatype.startsWith("application/vnd.") ) { // accept only types that look similar to own mediatypes m_xRootFolder->SetMediaType( aPackageMediatype ); - m_bMediaTypeFallbackUsed = true; + // if there is an encrypted inner package, there is no root + // document, because instead there is a package, and it is not + // an error + if (!m_xRootFolder->hasByName("encrypted-package")) + { + m_bMediaTypeFallbackUsed = true; + } } } else if ( !m_bForceRecovery ) { - // the mimetype stream should contain the information from manifest.xml - if ( m_xRootFolder->GetMediaType() != aPackageMediatype ) + // the mimetype stream should contain the same information as manifest.xml + OUString const mediaTypeXML(m_xRootFolder->hasByName("encrypted-package") + ? m_xRootFolder->doGetByName("encrypted-package").xPackageEntry->GetMediaType() + : m_xRootFolder->GetMediaType()); + if (mediaTypeXML != aPackageMediatype) + { throw ZipIOException( THROW_WHERE "mimetype conflicts with manifest.xml, \"" - + m_xRootFolder->GetMediaType() + "\" vs. \"" + + mediaTypeXML + "\" vs. \"" + aPackageMediatype + "\"" ); + } } m_xRootFolder->removeByName( sMimetype ); @@ -1269,6 +1280,8 @@ uno::Reference< io::XInputStream > ZipPackage::writeTempFile() static constexpr OUStringLiteral sFullPath(u"FullPath"); const bool bIsGpgEncrypt = m_aGpgProps.hasElements(); + // note: this is always created here (needed for GPG), possibly + // filtered out later in ManifestExport if ( m_nFormat == embed::StorageFormats::PACKAGE ) { uno::Sequence < PropertyValue > aPropSeq( diff --git a/svx/source/xml/xmlxtexp.cxx b/svx/source/xml/xmlxtexp.cxx index bd03967bd832..214d976b7d71 100644 --- a/svx/source/xml/xmlxtexp.cxx +++ b/svx/source/xml/xmlxtexp.cxx @@ -227,7 +227,13 @@ bool SvxXMLXTableExportComponent::save( if( !bToStorage || !xStorage.is() ) { // local URL -> SfxMedium route if( bSaveAsStorage ) + { + // ideally this should use a ZIP_STORAGE_FORMAT_STRING storage + // but changing it to that could cause problems loading the + // file with an old version of LO that expects to find in the + // user profile a PACKAGE_STORAGE_FORMAT_STRING storage xSubStorage = ::comphelper::OStorageHelper::GetStorageFromURL( rURL, eCreate ); + } else { pMedium.reset(new SfxMedium( rURL, StreamMode::WRITE | StreamMode::TRUNC ));
