From c2cf13da3bbf756ef3f78251c40d45fc23c27f36 Mon Sep 17 00:00:00 2001

From: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>

Date: Wed, 29 Apr 2020 07:42:24 +0200

Subject: [PATCH 1/6] backports to ease CVE backporting

Remove unnecessary if block

And format code inside

Change-Id: Ied0d98935134bf6f7bc8c929645ad5faac9affa3

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/93116

Tested-by: Jenkins

Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>

(cherry picked from commit cf36fe5eb41910c26d58fb25e54ccf2e0ee01365)

space out the namespace constant values

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88938

(cherry picked from commit 5352d45dd4a04f8f02cf7f6ad4169126d3b3586a)

Change-Id: I30f54bfc1389e91b18e4fee8b83e1b297419899b

---

 include/xmloff/xmlnmspe.hxx                   |  16 +--

 .../component/documentdigitalsignatures.cxx   | 130 +++++++++---------

 .../source/helper/xmlsignaturehelper.cxx      |   1 +

 3 files changed, 75 insertions(+), 72 deletions(-)

diff --git a/include/xmloff/xmlnmspe.hxx b/include/xmloff/xmlnmspe.hxx

index a00628b6b999..b079053c38d3 100644

--- a/include/xmloff/xmlnmspe.hxx

+++ b/include/xmloff/xmlnmspe.hxx

@@ -69,7 +69,7 @@ XML_NAMESPACE( XML_NAMESPACE_VERSIONS_LIST,   37U )

 // namespaces for odf extended formats

-#define XML_NAMESPACE_EXT_BASE 38U

+#define XML_NAMESPACE_EXT_BASE 50U

 #define XML_NAMESPACE_EXT( prefix, index ) \

 const sal_uInt16 prefix = (XML_NAMESPACE_EXT_BASE+index);

@@ -82,7 +82,7 @@ XML_NAMESPACE_EXT( XML_NAMESPACE_LO_EXT,           5U )

 // namespaces for OOo formats

-#define XML_NAMESPACE_OOO_BASE 44U

+#define XML_NAMESPACE_OOO_BASE 60U

 #define XML_NAMESPACE_OOO( prefix, index ) \

 const sal_uInt16 prefix = (XML_NAMESPACE_OOO_BASE+index);

@@ -100,7 +100,7 @@ XML_NAMESPACE_OOO( XML_NAMESPACE_CONFIG_OOO,        10U )

 XML_NAMESPACE_OOO( XML_NAMESPACE_FORM_OOO,          11U )

 XML_NAMESPACE_OOO( XML_NAMESPACE_SCRIPT_OOO,        12U )

-#define XML_NAMESPACE_COMPAT_BASE 57U

+#define XML_NAMESPACE_COMPAT_BASE 80U

 #define XML_NAMESPACE_COMPAT( prefix, index ) \

 const sal_uInt16 prefix = (XML_NAMESPACE_COMPAT_BASE+index);

@@ -108,14 +108,14 @@ XML_NAMESPACE_COMPAT( XML_NAMESPACE_SVG_COMPAT,      0U )

 XML_NAMESPACE_COMPAT( XML_NAMESPACE_FO_COMPAT,       1U )

 XML_NAMESPACE_COMPAT( XML_NAMESPACE_SMIL_COMPAT,     2U )

-#define XML_NAMESPACE_OASIS_BASE 60U

+#define XML_NAMESPACE_OASIS_BASE 90U

 #define XML_NAMESPACE_OASIS( prefix, index ) \

 const sal_uInt16 prefix = (XML_NAMESPACE_OASIS_BASE+index);

 XML_NAMESPACE_OASIS( XML_NAMESPACE_DB_OASIS,         0U )

 XML_NAMESPACE_OASIS( XML_NAMESPACE_REPORT_OASIS,     1U )

-#define XML_OLD_NAMESPACE_BASE 62U

+#define XML_OLD_NAMESPACE_BASE 100U

 #define XML_OLD_NAMESPACE( prefix, index ) \

 const sal_uInt16 prefix = (XML_OLD_NAMESPACE_BASE+index);

@@ -134,9 +134,9 @@ XML_OLD_NAMESPACE( XML_OLD_NAMESPACE_CHART,         10U )

 XML_OLD_NAMESPACE( XML_OLD_NAMESPACE_SMIL,          11U )

 // experimental namespaces

-XML_NAMESPACE( XML_NAMESPACE_FIELD,           100U )

-XML_NAMESPACE( XML_NAMESPACE_CSS3TEXT,        103U )  // CSS Text Level 3

-XML_NAMESPACE( XML_NAMESPACE_FORMX,           101U )  // form interop extensions

+XML_NAMESPACE( XML_NAMESPACE_FIELD,           120U )

+XML_NAMESPACE( XML_NAMESPACE_CSS3TEXT,        123U )  // CSS Text Level 3

+XML_NAMESPACE( XML_NAMESPACE_FORMX,           121U )  // form interop extensions

 #endif // INCLUDED_XMLOFF_XMLNMSPE_HXX

diff --git a/xmlsecurity/source/component/documentdigitalsignatures.cxx b/xmlsecurity/source/component/documentdigitalsignatures.cxx

index dcfaad0af773..52cb938a8e0a 100644

--- a/xmlsecurity/source/component/documentdigitalsignatures.cxx

+++ b/xmlsecurity/source/component/documentdigitalsignatures.cxx

@@ -500,85 +500,87 @@ DocumentDigitalSignatures::ImplVerifySignatures(

     Sequence< css::security::DocumentSignatureInformation > aInfos(nInfos);

     css::security::DocumentSignatureInformation* arInfos = aInfos.getArray();

-    if ( nInfos )

+    for (int n = 0; n < nInfos; ++n)

     {

-        for( int n = 0; n < nInfos; ++n )

-        {

-            DocumentSignatureAlgorithm mode = DocumentSignatureHelper::getDocumentAlgorithm(

-                m_sODFVersion, aSignInfos[n]);

-            const std::vector< OUString > aElementsToBeVerified =

-                DocumentSignatureHelper::CreateElementList(

-                rxStorage, eMode, mode);

+        DocumentSignatureAlgorithm mode

+            = DocumentSignatureHelper::getDocumentAlgorithm(m_sODFVersion, aSignInfos[n]);

+        const std::vector<OUString> aElementsToBeVerified

+            = DocumentSignatureHelper::CreateElementList(rxStorage, eMode, mode);

-            const SignatureInformation& rInfo = aSignInfos[n];

-            css::security::DocumentSignatureInformation& rSigInfo = arInfos[n];

+        const SignatureInformation& rInfo = aSignInfos[n];

+        css::security::DocumentSignatureInformation& rSigInfo = arInfos[n];

-            if (rInfo.ouGpgCertificate.isEmpty()) // X.509

+        if (rInfo.ouGpgCertificate.isEmpty()) // X.509

+        {

+            if (!rInfo.ouX509Certificate.isEmpty())

+                rSigInfo.Signer = xSecEnv->createCertificateFromAscii(rInfo.ouX509Certificate);

+            if (!rSigInfo.Signer.is())

+                rSigInfo.Signer = xSecEnv->getCertificate(

+                    rInfo.ouX509IssuerName,

+                    xmlsecurity::numericStringToBigInteger(rInfo.ouX509SerialNumber));

+

+            // On Windows checking the certificate path is buggy. It does name matching (issuer, subject name)

+            // to find the parent certificate. It does not take into account that there can be several certificates

+            // with the same subject name.

+            try

             {

-                if (!rInfo.ouX509Certificate.isEmpty())

-                    rSigInfo.Signer = xSecEnv->createCertificateFromAscii( rInfo.ouX509Certificate ) ;

-                if (!rSigInfo.Signer.is())

-                    rSigInfo.Signer = xSecEnv->getCertificate( rInfo.ouX509IssuerName,

-                                                               xmlsecurity::numericStringToBigInteger( rInfo.ouX509SerialNumber ) );

-

-                // On Windows checking the certificate path is buggy. It does name matching (issuer, subject name)

-                // to find the parent certificate. It does not take into account that there can be several certificates

-                // with the same subject name.

-

-                try {

-                    rSigInfo.CertificateStatus = xSecEnv->verifyCertificate(rSigInfo.Signer,

-                                                                            Sequence<Reference<css::security::XCertificate> >());

-                } catch (SecurityException& ) {

-                    OSL_FAIL("Verification of certificate failed");

-                    rSigInfo.CertificateStatus = css::security::CertificateValidity::INVALID;

-                }

+                rSigInfo.CertificateStatus = xSecEnv->verifyCertificate(

+                    rSigInfo.Signer, Sequence<Reference<css::security::XCertificate>>());

             }

-            else if (xGpgSecEnv.is()) // GPG

+            catch (SecurityException&)

             {

-                // TODO not ideal to retrieve cert by keyID, might

-                // collide, or PGPKeyID format might change - can't we

-                // keep the xCert itself in rInfo?

-                rSigInfo.Signer = xGpgSecEnv->getCertificate( rInfo.ouGpgKeyID, xmlsecurity::numericStringToBigInteger("") );

-                rSigInfo.CertificateStatus = xGpgSecEnv->verifyCertificate(rSigInfo.Signer,

-                                                                           Sequence<Reference<css::security::XCertificate> >());

+                OSL_FAIL("Verification of certificate failed");

+                rSigInfo.CertificateStatus = css::security::CertificateValidity::INVALID;

             }

+        }

+        else if (xGpgSecEnv.is()) // GPG

+        {

+            // TODO not ideal to retrieve cert by keyID, might

+            // collide, or PGPKeyID format might change - can't we

+            // keep the xCert itself in rInfo?

+            rSigInfo.Signer = xGpgSecEnv->getCertificate(

+                rInfo.ouGpgKeyID, xmlsecurity::numericStringToBigInteger(""));

+            rSigInfo.CertificateStatus = xGpgSecEnv->verifyCertificate(

+                rSigInfo.Signer, Sequence<Reference<css::security::XCertificate>>());

+        }

-            // Time support again (#i38744#)

-            Date aDate( rInfo.stDateTime.Day, rInfo.stDateTime.Month, rInfo.stDateTime.Year );

-            tools::Time aTime( rInfo.stDateTime.Hours, rInfo.stDateTime.Minutes,

-                        rInfo.stDateTime.Seconds, rInfo.stDateTime.NanoSeconds );

-            rSigInfo.SignatureDate = aDate.GetDate();

-            rSigInfo.SignatureTime = aTime.GetTime() / tools::Time::nanoPerCenti;

+        // Time support again (#i38744#)

+        Date aDate(rInfo.stDateTime.Day, rInfo.stDateTime.Month, rInfo.stDateTime.Year);

+        tools::Time aTime(rInfo.stDateTime.Hours, rInfo.stDateTime.Minutes,

+                          rInfo.stDateTime.Seconds, rInfo.stDateTime.NanoSeconds);

+        rSigInfo.SignatureDate = aDate.GetDate();

+        rSigInfo.SignatureTime = aTime.GetTime() / tools::Time::nanoPerCenti;

-            rSigInfo.SignatureIsValid = ( rInfo.nStatus == css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED );

+        rSigInfo.SignatureIsValid

+            = (rInfo.nStatus == css::xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED);

-            // Signature line info (ID + Images)

-            if (!rInfo.ouSignatureLineId.isEmpty())

-                rSigInfo.SignatureLineId = rInfo.ouSignatureLineId;

+        // Signature line info (ID + Images)

+        if (!rInfo.ouSignatureLineId.isEmpty())

+            rSigInfo.SignatureLineId = rInfo.ouSignatureLineId;

-            if (rInfo.aValidSignatureImage.is())

-                rSigInfo.ValidSignatureLineImage = rInfo.aValidSignatureImage;

+        if (rInfo.aValidSignatureImage.is())

+            rSigInfo.ValidSignatureLineImage = rInfo.aValidSignatureImage;

-            if (rInfo.aInvalidSignatureImage.is())

-                rSigInfo.InvalidSignatureLineImage = rInfo.aInvalidSignatureImage;

-

-            // OOXML intentionally doesn't sign metadata.

-            if ( rSigInfo.SignatureIsValid && aStreamHelper.nStorageFormat != embed::StorageFormats::OFOPXML)

-            {

-                 rSigInfo.SignatureIsValid =

-                      DocumentSignatureHelper::checkIfAllFilesAreSigned(

-                      aElementsToBeVerified, rInfo, mode);

-            }

-            if (eMode == DocumentSignatureMode::Content)

-            {

-                if (aStreamHelper.nStorageFormat == embed::StorageFormats::OFOPXML)

-                    rSigInfo.PartialDocumentSignature = true;

-                else

-                    rSigInfo.PartialDocumentSignature = !DocumentSignatureHelper::isOOo3_2_Signature(aSignInfos[n]);

-            }

+        if (rInfo.aInvalidSignatureImage.is())

+            rSigInfo.InvalidSignatureLineImage = rInfo.aInvalidSignatureImage;

+        // OOXML intentionally doesn't sign metadata.

+        if (rSigInfo.SignatureIsValid

+            && aStreamHelper.nStorageFormat != embed::StorageFormats::OFOPXML)

+        {

+            rSigInfo.SignatureIsValid = DocumentSignatureHelper::checkIfAllFilesAreSigned(

+                aElementsToBeVerified, rInfo, mode);

+        }

+        if (eMode == DocumentSignatureMode::Content)

+        {

+            if (aStreamHelper.nStorageFormat == embed::StorageFormats::OFOPXML)

+                rSigInfo.PartialDocumentSignature = true;

+            else

+                rSigInfo.PartialDocumentSignature

+                    = !DocumentSignatureHelper::isOOo3_2_Signature(aSignInfos[n]);

         }

     }

+

     return aInfos;

 }

diff --git a/xmlsecurity/source/helper/xmlsignaturehelper.cxx b/xmlsecurity/source/helper/xmlsignaturehelper.cxx

index 6ec834053a17..22c056e70da1 100644

--- a/xmlsecurity/source/helper/xmlsignaturehelper.cxx

+++ b/xmlsecurity/source/helper/xmlsignaturehelper.cxx

@@ -402,6 +402,7 @@ bool XMLSignatureHelper::ReadAndVerifySignatureStorageStream(const css::uno::Ref

     catch(const uno::Exception&)

     {

         DBG_UNHANDLED_EXCEPTION("xmlsecurity.helper");

+        mbError = true;

     }

     mpXSecController->releaseSignatureReader();

-- 

2.32.0