From 96208a1c545e490014800289c74cc555a20c4507 Mon Sep 17 00:00:00 2001 From: Oleg Korshul Date: Wed, 12 Jul 2023 16:33:42 +0300 Subject: [PATCH] Fix bug 59649 --- .../xmlsec/src/include/OOXMLVerifier.h | 1 + DesktopEditor/xmlsec/src/ooxmlsignature.pro | 1 + DesktopEditor/xmlsec/src/src/OOXMLSigner.cpp | 74 +------------- .../xmlsec/src/src/OOXMLVerifier.cpp | 97 +++++++++++++++++- DesktopEditor/xmlsec/src/src/XmlRels.h | 16 ++- DesktopEditor/xmlsec/src/src/XmlTransform.h | 22 +---- DesktopEditor/xmlsec/src/src/common.h | 99 +++++++++++++++++++ 7 files changed, 214 insertions(+), 96 deletions(-) create mode 100644 DesktopEditor/xmlsec/src/src/common.h diff --git a/DesktopEditor/xmlsec/src/include/OOXMLVerifier.h b/DesktopEditor/xmlsec/src/include/OOXMLVerifier.h index 761bfddecb..b829edc6da 100644 --- a/DesktopEditor/xmlsec/src/include/OOXMLVerifier.h +++ b/DesktopEditor/xmlsec/src/include/OOXMLVerifier.h @@ -7,6 +7,7 @@ #define OOXML_SIGNATURE_INVALID 1 #define OOXML_SIGNATURE_NOTSUPPORTED 2 #define OOXML_SIGNATURE_BAD 3 +#define OOXML_SIGNATURE_PARTIALLY 4 class COOXMLSignature_private; class OPENSSL_DECL COOXMLSignature diff --git a/DesktopEditor/xmlsec/src/ooxmlsignature.pro b/DesktopEditor/xmlsec/src/ooxmlsignature.pro index e6c3b626ce..313a483dd1 100644 --- a/DesktopEditor/xmlsec/src/ooxmlsignature.pro +++ b/DesktopEditor/xmlsec/src/ooxmlsignature.pro @@ -27,6 +27,7 @@ HEADERS += \ src/XmlTransform.h SOURCES += \ + src/common.h \ src/XmlTransform.cpp \ src/CertificateCommon.cpp \ src/OOXMLSigner.cpp \ diff --git a/DesktopEditor/xmlsec/src/src/OOXMLSigner.cpp b/DesktopEditor/xmlsec/src/src/OOXMLSigner.cpp index 43d1b77bc8..bad36cf497 100644 --- a/DesktopEditor/xmlsec/src/src/OOXMLSigner.cpp +++ b/DesktopEditor/xmlsec/src/src/OOXMLSigner.cpp @@ -1,12 +1,7 @@ #include "./../include/OOXMLSigner.h" -#include "../../../../OfficeUtils/src/ZipFolder.h" -#include "./XmlTransform.h" -#include -#include -#include -#include "./../include/CertificateCommon.h" +#include "./common.h" -class COOXMLSigner_private +class COOXMLSigner_private : public CSignFolderFiles { public: ICertificate* m_certificate; @@ -14,10 +9,6 @@ public: std::wstring m_date; - std::map m_content_types; - std::vector m_rels; - std::vector m_files; - NSStringUtils::CStringBuilderA m_signed_info; std::wstring m_image_valid; @@ -263,31 +254,7 @@ public: void ParseContentTypes() { - XmlUtils::CXmlNode oNode = m_pFolder->getNodeFromFile(L"/[Content_Types].xml"); - - XmlUtils::CXmlNodes nodesDefaults; - oNode.GetNodes(L"Default", nodesDefaults); - - XmlUtils::CXmlNodes nodesOverrides; - oNode.GetNodes(L"Override", nodesOverrides); - - int nCount = nodesDefaults.GetCount(); - for (int i = 0; i < nCount; ++i) - { - XmlUtils::CXmlNode node; - nodesDefaults.GetAt(i, node); - - m_content_types.insert(std::pair(node.GetAttribute("Extension"), node.GetAttribute("ContentType"))); - } - - nCount = nodesOverrides.GetCount(); - for (int i = 0; i < nCount; ++i) - { - XmlUtils::CXmlNode node; - nodesOverrides.GetAt(i, node); - - m_content_types.insert(std::pair(node.GetAttribute("PartName"), node.GetAttribute("ContentType"))); - } + Folder_ParseContentTypes(m_pFolder); } void Parse() @@ -295,39 +262,8 @@ public: // 1) Parse Content_Types.xml ParseContentTypes(); - // 2) Parse files in directory - std::vector files = m_pFolder->getFiles(L"", true); - - // 3) Check each file - std::wstring sFolder = L""; - for (std::vector::iterator i = files.begin(); i != files.end(); i++) - { - std::wstring sCheckFile = *i; - - // make cool filename - sCheckFile = m_pFolder->getLocalFilePath(sCheckFile); - - // check needed file - if (0 == sCheckFile.find(L"_xmlsignatures") || - 0 == sCheckFile.find(L"docProps") || - 0 == sCheckFile.find(L"[Content_Types].xml") || - 0 == sCheckFile.find(L"[trash]")) - continue; - - // check rels and add to needed array - std::wstring::size_type posExt = sCheckFile.rfind(L"."); - if (std::wstring::npos == posExt) - continue; - - std::wstring sExt = sCheckFile.substr(posExt + 1); - if (sExt == L"rels") - m_rels.push_back(sCheckFile); - else - m_files.push_back(sCheckFile); - } - - std::sort(m_rels.begin(), m_rels.end()); - std::sort(m_files.begin(), m_files.end()); + // 2) Parse files & check each file in directory + Folder_Parse(m_pFolder); } void WriteRelsReferences(NSStringUtils::CStringBuilder& builder) diff --git a/DesktopEditor/xmlsec/src/src/OOXMLVerifier.cpp b/DesktopEditor/xmlsec/src/src/OOXMLVerifier.cpp index e01c207b71..498db387f5 100644 --- a/DesktopEditor/xmlsec/src/src/OOXMLVerifier.cpp +++ b/DesktopEditor/xmlsec/src/src/OOXMLVerifier.cpp @@ -1,7 +1,5 @@ -#include "./XmlTransform.h" #include "./../include/OOXMLVerifier.h" -#include "../../../../OfficeUtils/src/ZipFolder.h" -#include "./../include/CertificateCommon.h" +#include "common.h" class COOXMLSignature_private { @@ -20,6 +18,8 @@ public: XmlUtils::CXmlNode m_node; // signature file + std::set m_arFilesInManifest; + class CXmlStackNamespaces { public: @@ -139,6 +139,38 @@ public: RELEASEOBJECT(m_cert); } +public: + void AddInvalidType(const int type) + { + switch (type) + { + case OOXML_SIGNATURE_INVALID: + case OOXML_SIGNATURE_BAD: + case OOXML_SIGNATURE_NOTSUPPORTED: + { + // critical + m_valid = type; + break; + } + default: + { + switch (m_valid) + { + case OOXML_SIGNATURE_INVALID: + case OOXML_SIGNATURE_BAD: + case OOXML_SIGNATURE_NOTSUPPORTED: + { + break; + } + default: + { + m_valid = type; + break; + } + } + } + } + } public: int GetValid() { @@ -290,6 +322,64 @@ public: if (OPEN_SSL_WARNING_NOVERIFY == nCertVerify) m_valid = OOXML_SIGNATURE_INVALID; } + + // 7) Test on partically + if (m_valid == OOXML_SIGNATURE_VALID) + { + CSignFolderFiles oFiles; + oFiles.Folder_Parse(m_pFolder, true); + + // 1) Все рельсы должны быть подписаны - иначе подпись не валидна + for (std::vector::const_iterator i = oFiles.m_rels.begin(); i != oFiles.m_rels.end(); i++) + { + if (m_arFilesInManifest.find(*i) == m_arFilesInManifest.end()) + { + m_valid = OOXML_SIGNATURE_INVALID; + break; + } + } + + if (m_valid == OOXML_SIGNATURE_VALID) + { + // 2) Парсим все рельсы + for (std::vector::const_iterator i = oFiles.m_rels.begin(); i != oFiles.m_rels.end(); i++) + { + std::wstring sFile = *i; + + CManifestFileInfo oInfo; + oInfo.m_pFolder = m_pFolder; + oInfo.SetFilePath(sFile); + + std::string sXml = m_pFolder->readXml(sFile); + COOXMLRelationships _rels(sXml, &oInfo); + + for (std::vector::const_iterator relsIter = _rels.rels.begin(); relsIter != _rels.rels.end(); relsIter++) + { + const COOXMLRelationship& curRel = *relsIter; + + if (curRel.target_mode == L"Internal" && !CSignFolderFiles::CheckNeedSign(curRel.target)) + continue; + + std::wstring sFullPath = oInfo.GetHeadPath(curRel.target); + + // если внутренний файл отсутствует - не валидная подпись + if (curRel.target_mode == L"Internal") + { + if (!m_pFolder->exists(sFullPath)) + m_valid = OOXML_SIGNATURE_INVALID; + else + { + // если файл в списке, но не подписан - то подпись частичная. + if (m_arFilesInManifest.find(sFullPath) == m_arFilesInManifest.end()) + { + AddInvalidType(OOXML_SIGNATURE_PARTIALLY); + } + } + } + } + } + } + } } XmlUtils::CXmlNode GetObjectById(std::string sId) @@ -399,6 +489,7 @@ public: return OOXML_SIGNATURE_INVALID; sFile = sFile.substr(0, nPos); + m_arFilesInManifest.insert(sFile); if (!m_pFolder->exists(sFile)) return OOXML_SIGNATURE_INVALID; diff --git a/DesktopEditor/xmlsec/src/src/XmlRels.h b/DesktopEditor/xmlsec/src/src/XmlRels.h index f0e4081c54..e9daab3d90 100644 --- a/DesktopEditor/xmlsec/src/src/XmlRels.h +++ b/DesktopEditor/xmlsec/src/src/XmlRels.h @@ -5,6 +5,7 @@ #include "../../../common/File.h" #include "../../../common/Directory.h" #include "../../../../OfficeUtils/src/ZipFolder.h" +#include class CManifestFileInfo { @@ -38,7 +39,7 @@ public: void CheckAliasExist(const std::wstring& sFile) { - if (!m_pFolder->exists(m_sAliasDirectory + L"/" + sFile)) + if (!m_pFolder->exists(GetHeadPath(sFile))) ++m_nCountUnexistedFile; } @@ -46,6 +47,11 @@ public: { return (0 != m_nCountUnexistedFile) ? true : false; } + + std::wstring GetHeadPath(const std::wstring& sFile) + { + return m_sAliasDirectory + L"/" + sFile; + } }; class COOXMLRelationship @@ -117,7 +123,7 @@ public: { } - COOXMLRelationships(const std::string& xml, CManifestFileInfo* pFileInfo, std::map* check_need = NULL) + COOXMLRelationships(const std::string& xml, CManifestFileInfo* pFileInfo, std::set* check_need = NULL) { m_pFileInfo = pFileInfo; XmlUtils::CXmlNode oNode; @@ -127,11 +133,11 @@ public: FromXmlNode(oNode, check_need); } - COOXMLRelationships(CManifestFileInfo* pFileInfo, std::map* check_need = NULL) + COOXMLRelationships(CManifestFileInfo* pFileInfo, std::set* check_need = NULL) { m_pFileInfo = pFileInfo; - if (!m_pFileInfo || NULL != m_pFileInfo->m_pFolder) + if (!m_pFileInfo || NULL == m_pFileInfo->m_pFolder) return; XmlUtils::CXmlNode oNode = m_pFileInfo->m_pFolder->getNodeFromFile(m_pFileInfo->GetFilePath()); @@ -141,7 +147,7 @@ public: FromXmlNode(oNode, check_need); } - void FromXmlNode(XmlUtils::CXmlNode& oNode, std::map* check_need = NULL) + void FromXmlNode(XmlUtils::CXmlNode& oNode, std::set* check_need = NULL) { XmlUtils::CXmlNodes oNodes; if (!oNode.GetNodes(L"Relationship", oNodes)) diff --git a/DesktopEditor/xmlsec/src/src/XmlTransform.h b/DesktopEditor/xmlsec/src/src/XmlTransform.h index a939473291..fc384c2df7 100644 --- a/DesktopEditor/xmlsec/src/src/XmlTransform.h +++ b/DesktopEditor/xmlsec/src/src/XmlTransform.h @@ -28,7 +28,7 @@ class CXmlTransformRelationship : public IXmlTransform { protected: CManifestFileInfo* m_pManifestFileInfo; - std::map m_arIds; + std::set m_arIds; public: CXmlTransformRelationship(CManifestFileInfo* pManifestFileInfo) : IXmlTransform() @@ -39,23 +39,7 @@ public: virtual std::string Transform(const std::string& xml) { - std::map* checker = &m_arIds; - - // для некоторых путей не считаем валидными добавления в rels после подписи - if (m_pManifestFileInfo) - { - std::wstring& sFile = m_pManifestFileInfo->GetFilePath(); - if (0 == sFile.find(L"/word/") || - 0 == sFile.find(L"/ppt/") || - 0 == sFile.find(L"/xl/")) - { - // https://bugzilla.onlyoffice.com/show_bug.cgi?id=59649 - checker = NULL; - } - } - - - COOXMLRelationships _rels(xml, m_pManifestFileInfo, checker); + COOXMLRelationships _rels(xml, m_pManifestFileInfo, &m_arIds); return U_TO_UTF8(_rels.GetXml()); } @@ -72,7 +56,7 @@ public: std::wstring sType = _node.GetAttribute("SourceId"); if (!sType.empty()) - m_arIds.insert(std::pair(sType, true)); + m_arIds.insert(sType); } } }; diff --git a/DesktopEditor/xmlsec/src/src/common.h b/DesktopEditor/xmlsec/src/src/common.h new file mode 100644 index 0000000000..01160ded5f --- /dev/null +++ b/DesktopEditor/xmlsec/src/src/common.h @@ -0,0 +1,99 @@ +#pragma once + +#include "./../include/CertificateCommon.h" +#include "../../../../OfficeUtils/src/ZipFolder.h" +#include "./XmlTransform.h" +#include +#include +#include + +class CSignFolderFiles +{ +public: + std::map m_content_types; + std::vector m_rels; + std::vector m_files; + +public: + CSignFolderFiles() {} + ~CSignFolderFiles() {} + +public: + static bool CheckNeedSign(const std::wstring& sCheckFile) + { + if (0 == sCheckFile.find(L"_xmlsignatures") || + 0 == sCheckFile.find(L"docProps") || + 0 == sCheckFile.find(L"[Content_Types].xml") || + 0 == sCheckFile.find(L"[trash]")) + return false; + return true; + } + + void Folder_Parse(IFolder* pFolder, bool bIsAddSlash = false) + { + // 1) Parse files in directory + std::vector files = pFolder->getFiles(L"", true); + + // 2) Check each file + std::wstring sFolder = L""; + for (std::vector::iterator i = files.begin(); i != files.end(); i++) + { + std::wstring sCheckFile = *i; + + // make cool filename + sCheckFile = pFolder->getLocalFilePath(sCheckFile); + + // check needed file + if (!CheckNeedSign(sCheckFile)) + continue; + + // check rels and add to needed array + std::wstring::size_type posExt = sCheckFile.rfind(L"."); + if (std::wstring::npos == posExt) + continue; + + std::wstring sExt = sCheckFile.substr(posExt + 1); + + if (bIsAddSlash) + sCheckFile = L"/" + sCheckFile; + + if (sExt == L"rels") + m_rels.push_back(sCheckFile); + else + m_files.push_back(sCheckFile); + } + + std::sort(m_rels.begin(), m_rels.end()); + std::sort(m_files.begin(), m_files.end()); + } + + void Folder_ParseContentTypes(IFolder* pFolder) + { + XmlUtils::CXmlNode oNode = pFolder->getNodeFromFile(L"/[Content_Types].xml"); + + XmlUtils::CXmlNodes nodesDefaults; + oNode.GetNodes(L"Default", nodesDefaults); + + XmlUtils::CXmlNodes nodesOverrides; + oNode.GetNodes(L"Override", nodesOverrides); + + int nCount = nodesDefaults.GetCount(); + for (int i = 0; i < nCount; ++i) + { + XmlUtils::CXmlNode node; + nodesDefaults.GetAt(i, node); + + m_content_types.insert(std::pair(node.GetAttribute("Extension"), node.GetAttribute("ContentType"))); + } + + nCount = nodesOverrides.GetCount(); + for (int i = 0; i < nCount; ++i) + { + XmlUtils::CXmlNode node; + nodesOverrides.GetAt(i, node); + + m_content_types.insert(std::pair(node.GetAttribute("PartName"), node.GetAttribute("ContentType"))); + } + } +}; +