From 6a7bd10fd46827ed0ad6a951d35e9aad4d5be912 Mon Sep 17 00:00:00 2001 From: Caolan McNamara Date: Sat, 25 Mar 2023 18:15:47 +0000 Subject: [PATCH] Subject: CVE-2021-25636: only use X509Data MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 (cherry picked from commit be446d81e07b5499152efeca6ca23034e51ea5ff) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127178 Reviewed-by: Adolfo Jayme Barrientos (cherry picked from commit b0404f80577de9ff69e58390c6f6ef949fdb0139) Signed-off-by: Bastien Roucariès 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 Gbp-Pq: Name 0066-Subject-CVE-2021-25636-only-use-X509Data.patch --- .../source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx | 6 ++++++ xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx index dfa9c4ad494..5251d928752 100644 --- a/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx +++ b/xmlsecurity/source/xmlsec/mscrypt/xmlsignature_mscryptimpl.cxx @@ -20,6 +20,8 @@ #include #include +#include + #include #include @@ -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 f4b1364f52d..c3b6d7aded7 100644 --- a/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx +++ b/xmlsecurity/source/xmlsec/nss/xmlsignature_nssimpl.cxx @@ -21,6 +21,8 @@ #include #include +#include + #include #include #include @@ -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 ); -- 2.30.2