Subject: CVE-2021-25636: only use X509Data

LibreOffice supports digital signatures of ODF documents and macros
within documents, presenting visual aids that no alteration of the
document occurred since the last signing and that the signature is
valid. An Improper Certificate Validation vulnerability in LibreOffice
allowed an attacker to create a digitally signed ODF document, by
manipulating the documentsignatures.xml or macrosignatures.xml stream
within the document to contain both "X509Data" and "KeyValue" children
of the "KeyInfo" tag, which when opened caused LibreOffice to verify
using the "KeyValue" but to report verification with the unrelated
"X509Data" value.

Change-Id: I52e6588f5fac04bb26d77c1f3af470db73e41f72
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127193
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit be446d81e07b5499152efeca6ca23034e51ea5ff)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127178
Reviewed-by: Adolfo Jayme Barrientos <fitojb@ubuntu.com>
(cherry picked from commit b0404f80577de9ff69e58390c6f6ef949fdb0139)
Signed-off-by: Bastien Roucariès <rouca@debian.org>
bug-debian-security: https://security-tracker.debian.org/tracker/CVE-2021-25636
bug-redhat: https://bugzilla.redhat.com/show_bug.cgi?id=2056955
origin: https://gitlab.com/redhat/centos-stream/rpms/libreoffice/-/raw/c8s/0001-CVE-2021-25636.patch
bug: https://www.libreoffice.org/about-us/security/advisories/CVE-2021-25636
Signed-off-by: Bastien Roucariès <rouca@debian.org>
Gbp-Pq: Name 0066-Subject-CVE-2021-25636-only-use-X509Data.patch

diff --git a/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx
index dfa9c4ad494b4c35c5cd2d3c85b23a23e815b354..5251d928752565f5dee5d814441be85bde4572c2 100644 (file)
--- a/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx
+++ b/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx
@@ -20,6 +20,8 @@
 #include <sal/config.h>
 #include <rtl/uuid.h>
 
+#include <xmlsec/mscng/x509.h>
+
 #include <com/sun/star/xml/crypto/SecurityOperationStatus.hpp>
 #include <com/sun/star/xml/crypto/XXMLSignature.hpp>
 
@@ -228,6 +230,10 @@ SAL_CALL XMLSignature_MSCryptImpl::validate(
     // We do certificate verification ourselves.
     pDsigCtx->keyInfoReadCtx.flags |= XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS;
 
+    // limit possible key data to valid X509 certificates only, no KeyValues
+    if (xmlSecPtrListAdd(&(pDsigCtx->keyInfoReadCtx.enabledKeyData), BAD_CAST xmlSecMSCngKeyDataX509GetKlass()) < 0)
+        throw RuntimeException("failed to limit allowed key data");
+
     //Verify signature
     //The documentation says that the signature is only valid if the return value is 0 (that is, not < 0)
     //AND pDsigCtx->status == xmlSecDSigStatusSucceeded. That is, we must not make any assumptions, if

diff --git a/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx b/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx
index f4b1364f52dddbaac6f0e2568d23acc37d549ff0..c3b6d7aded7d46a81f6bb7a43509f0d2bc5faa33 100644 (file)
--- a/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx
+++ b/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx
@@ -21,6 +21,8 @@
 #include <rtl/uuid.h>
 
 #include <xmlsec/xmldocumentwrapper_xmlsecimpl.hxx>
+#include <xmlsec/nss/x509.h>
+
 #include <xmlelementwrapper_xmlsecimpl.hxx>
 #include <xmlsec/xmlstreamio.hxx>
 #include <xmlsec/errorcallback.hxx>
@@ -242,6 +244,10 @@ SAL_CALL XMLSignature_NssImpl::validate(
         // We do certificate verification ourselves.
         pDsigCtx->keyInfoReadCtx.flags |= XMLSEC_KEYINFO_FLAGS_X509DATA_DONT_VERIFY_CERTS;
 
+        // limit possible key data to valid X509 certificates only, no KeyValues
+        if (xmlSecPtrListAdd(&(pDsigCtx->keyInfoReadCtx.enabledKeyData), BAD_CAST xmlSecNssKeyDataX509GetKlass()) < 0)
+            throw RuntimeException("failed to limit allowed key data");
+
         //Verify signature
         int rs = xmlSecDSigCtxVerify( pDsigCtx , pNode );