From: Markus Koschany Date: Sun, 14 Nov 2021 18:59:23 +0100 Subject: CVE-2021-40690 Bug-Debian: https://bugs.debian.org/994569 Origin: https://github.com/apache/santuario-xml-security-java/commit/56ec2160c161c6bc41e5c297eed077e8ecece312 Origin: https://github.com/apache/santuario-xml-security-java/commit/c3410a7fbecaa7dca61361d887f801b9e6c66c15 --- .../xml/dsig/internal/dom/DOMURIDereferencer.java | 23 ++++++++++++--------- .../xml/security/encryption/XMLCipherInput.java | 13 +++++++++--- .../implementations/KeyInfoReferenceResolver.java | 15 +++++++++++--- .../implementations/RetrievalMethodResolver.java | 24 ++++++++++++++-------- .../security/resource/xmlsecurity_en.properties | 3 ++- .../xml/security/signature/XMLSignatureInput.java | 2 +- .../transforms/implementations/TransformXPath.java | 6 +----- .../security/utils/resolver/ResourceResolver.java | 24 ++++++++++++++++++++++ .../crypto/test/dsig/CreateBaltimore23Test.java | 1 + .../keyresolver/KeyInfoReferenceResolverTest.java | 22 ++++++++++++++++++++ .../KeyInfoReference-RSA-RetrievalMethod.xml | 22 ++++++++++++++++++++ 11 files changed, 124 insertions(+), 31 deletions(-) create mode 100644 src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml diff --git a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java index 1d7be4d..bbde878 100644 --- a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java +++ b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java @@ -106,17 +106,20 @@ public final class DOMURIDereferencer implements URIDereferencer { } } - try { - ResourceResolver apacheResolver = - ResourceResolver.getInstance(uriAttr, baseURI, secVal); - XMLSignatureInput in = apacheResolver.resolve(uriAttr, baseURI, secVal); - if (in.isOctetStream()) { - return new ApacheOctetStreamData(in); - } else { - return new ApacheNodeSetData(in); + if (uriRef instanceof javax.xml.crypto.dsig.Reference || ResourceResolver.isURISafeToResolve(uriAttr, baseURI)) { + try { + ResourceResolver apacheResolver = + ResourceResolver.getInstance(uriAttr, baseURI, secVal); + XMLSignatureInput in = apacheResolver.resolve(uriAttr, baseURI, secVal); + if (in.isOctetStream()) { + return new ApacheOctetStreamData(in); + } else { + return new ApacheNodeSetData(in); + } + } catch (Exception e) { + throw new URIReferenceException(e); } - } catch (Exception e) { - throw new URIReferenceException(e); } + throw new URIReferenceException("URI " + uri + " is forbidden"); } } diff --git a/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java b/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java index 003b402..87d2ac9 100644 --- a/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java +++ b/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java @@ -127,9 +127,16 @@ public class XMLCipherInput { XMLSignatureInput input = null; try { - ResourceResolver resolver = - ResourceResolver.getInstance(uriAttr, null, secureValidation); - input = resolver.resolve(uriAttr, null, secureValidation); + if (ResourceResolver.isURISafeToResolve(uriAttr, null)) { + ResourceResolver resolver = + ResourceResolver.getInstance(uriAttr, null, secureValidation); + input = resolver.resolve(uriAttr, null, secureValidation); + } else { + String uriToResolve = uriAttr != null ? uriAttr.getValue() : null; + Object[] exArgs = {uriToResolve != null ? uriToResolve : "null", null}; + + throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, null); + } } catch (ResourceResolverException ex) { throw new XMLEncryptionException(ex); } diff --git a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java index d0dae9b..4f3a63f 100644 --- a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java +++ b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java @@ -38,6 +38,7 @@ import org.apache.xml.security.signature.XMLSignatureInput; import org.apache.xml.security.utils.Constants; import org.apache.xml.security.utils.XMLUtils; import org.apache.xml.security.utils.resolver.ResourceResolver; +import org.apache.xml.security.utils.resolver.ResourceResolverException; import org.w3c.dom.Attr; import org.w3c.dom.Element; import org.xml.sax.SAXException; @@ -197,6 +198,7 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi { validateReference(referentElement); KeyInfo referent = new KeyInfo(referentElement, baseURI); + referent.setSecureValidation(secureValidation); referent.addStorageResolver(storage); return referent; } @@ -215,7 +217,7 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi { } KeyInfo referent = new KeyInfo(referentElement, ""); - if (referent.containsKeyInfoReference()) { + if (referent.containsKeyInfoReference() || referent.containsRetrievalMethod()) { if (secureValidation) { throw new XMLSecurityException("KeyInfoReferenceResolver.InvalidReferentElement.ReferenceWithSecure"); } else { @@ -239,8 +241,15 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi { */ private XMLSignatureInput resolveInput(Attr uri, String baseURI, boolean secureValidation) throws XMLSecurityException { - ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation); - return resRes.resolve(uri, baseURI, secureValidation); + if (ResourceResolver.isURISafeToResolve(uri, baseURI)) { + ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation); + return resRes.resolve(uri, baseURI, secureValidation); + } + + String uriToResolve = uri != null ? uri.getValue() : null; + Object[] exArgs = { uriToResolve != null ? uriToResolve : "null", baseURI }; + + throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, baseURI); } /** diff --git a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java index 811dcc6..a528e0f 100644 --- a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java +++ b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java @@ -45,6 +45,7 @@ import org.apache.xml.security.transforms.Transforms; import org.apache.xml.security.utils.Constants; import org.apache.xml.security.utils.XMLUtils; import org.apache.xml.security.utils.resolver.ResourceResolver; +import org.apache.xml.security.utils.resolver.ResourceResolverException; import org.w3c.dom.Attr; import org.w3c.dom.Element; import org.w3c.dom.Node; @@ -299,15 +300,22 @@ public class RetrievalMethodResolver extends KeyResolverSpi { Attr uri = rm.getURIAttr(); // Apply the transforms Transforms transforms = rm.getTransforms(); - ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation); - XMLSignatureInput resource = resRes.resolve(uri, baseURI, secureValidation); - if (transforms != null) { - if (log.isDebugEnabled()) { - log.debug("We have Transforms"); + if (ResourceResolver.isURISafeToResolve(uri, baseURI)) { + ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation); + XMLSignatureInput resource = resRes.resolve(uri, baseURI, secureValidation); + if (transforms != null) { + if (log.isDebugEnabled()) { + log.debug("We have Transforms"); + resource = transforms.performTransforms(resource); + } } - resource = transforms.performTransforms(resource); - } - return resource; + return resource; + } + + String uriToResolve = uri != null ? uri.getValue() : null; + Object[] exArgs = { uriToResolve != null ? uriToResolve : "null", baseURI }; + + throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, baseURI); } /** diff --git a/src/main/java/org/apache/xml/security/resource/xmlsecurity_en.properties b/src/main/java/org/apache/xml/security/resource/xmlsecurity_en.properties index 4ae18fe..8e8461f 100644 --- a/src/main/java/org/apache/xml/security/resource/xmlsecurity_en.properties +++ b/src/main/java/org/apache/xml/security/resource/xmlsecurity_en.properties @@ -124,6 +124,7 @@ signature.Transform.ForbiddenTransform = Transform {0} is forbidden when secure signature.Transform.NotYetImplemented = Transform {0} not yet implemented signature.Transform.NullPointerTransform = Null pointer as URI. Programming bug? signature.Transform.UnknownTransform = Unknown transformation. No handler installed for URI {0} +signature.Transform.XPathError = Error evaluating XPath expression signature.Transform.node = Current Node: {0} signature.Transform.nodeAndType = Current Node: {0}, type: {1} signature.Util.BignumNonPositive = bigInteger.signum() must be positive @@ -193,4 +194,4 @@ stax.signature.keyNameMissing = KeyName not configured. stax.keyNotFoundForName = No key configured for KeyName: {0} stax.keyTypeNotSupported = Key of type {0} not supported for a KeyName lookup stax.idsetbutnotgenerated = An Id attribute is specified, but Id generation is disabled -stax.idgenerationdisablewithmultipleparts = Id generation must not be disabled when multiple parts need signing \ No newline at end of file +stax.idgenerationdisablewithmultipleparts = Id generation must not be disabled when multiple parts need signing diff --git a/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java b/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java index f3a099f..9a25bba 100644 --- a/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java +++ b/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java @@ -547,7 +547,7 @@ public class XMLSignatureInput { convertToNodes(); } catch (Exception e) { throw new XMLSecurityRuntimeException( - "signature.XMLSignatureInput.nodesetReference", e + "signature.XMLSignatureInput.nodesetReference" ); } } diff --git a/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java b/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java index d5b87d5..aa62028 100644 --- a/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java +++ b/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java @@ -144,11 +144,7 @@ public class TransformXPath extends TransformSpi { } return 0; } catch (TransformerException e) { - Object[] eArgs = {currentNode}; - throw new XMLSecurityRuntimeException("signature.Transform.node", eArgs, e); - } catch (Exception e) { - Object[] eArgs = {currentNode, currentNode.getNodeType()}; - throw new XMLSecurityRuntimeException("signature.Transform.nodeAndType",eArgs, e); + throw new XMLSecurityRuntimeException("signature.Transform.XPathError"); } } diff --git a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java index 7e134fe..b969a86 100644 --- a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java +++ b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java @@ -18,6 +18,8 @@ */ package org.apache.xml.security.utils.resolver; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -39,6 +41,11 @@ import org.w3c.dom.Attr; */ public class ResourceResolver { + + private static boolean allowUnsafeResourceResolving = + AccessController.doPrivileged( + (PrivilegedAction) () -> Boolean.getBoolean("org.apache.xml.security.allowUnsafeResourceResolving")); + private static org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(ResourceResolver.class); @@ -336,6 +343,23 @@ public class ResourceResolver { return resolverSpi.understandsProperty(propertyToTest); } + public static boolean isURISafeToResolve(Attr uriAttr, String baseUri) { + if (allowUnsafeResourceResolving) { + return true; + } + String uriToResolve = uriAttr != null ? uriAttr.getValue() : null; + if (uriToResolve != null) { + if (uriToResolve.startsWith("file:") || uriToResolve.startsWith("http:")) { + return false; + } + if (!uriToResolve.isEmpty() && uriToResolve.charAt(0) != '#' && + baseUri != null && (baseUri.startsWith("file:") || baseUri.startsWith("http:"))) { + return false; + } + } + return true; + } + /** * Method canResolve * diff --git a/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java b/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java index accf6a3..08ef412 100644 --- a/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java +++ b/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java @@ -79,6 +79,7 @@ public class CreateBaltimore23Test extends org.junit.Assert { private final URIDereferencer ud; static { + System.setProperty("org.apache.xml.security.allowUnsafeResourceResolving", "true"); Security.insertProviderAt (new org.apache.jcp.xml.dsig.internal.dom.XMLDSigRI(), 1); } diff --git a/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java b/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java index eb3fe5d..dbcf65e 100644 --- a/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java +++ b/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java @@ -128,6 +128,19 @@ public class KeyInfoReferenceResolverTest extends Assert { assertNull(keyInfo.getPublicKey()); } + @org.junit.Test + public void testKeyInfoReferenceToRetrievalMethodNotAllowed() throws Exception { + Document doc = loadXML("KeyInfoReference-RSA-RetrievalMethod.xml"); + markKeyInfoIdAttrs(doc); + markEncodedKeyValueIdAttrs(doc); + + Element referenceElement = doc.getElementById("theReference"); + assertNotNull(referenceElement); + + KeyInfo keyInfo = new KeyInfo(referenceElement, ""); + assertNull(keyInfo.getPublicKey()); + } + // Utility methods private String getControlFilePath(String fileName) { @@ -163,4 +176,13 @@ public class KeyInfoReferenceResolverTest extends Assert { } } + private void markEncodedKeyValueIdAttrs(Document doc) { + NodeList nl = doc.getElementsByTagNameNS(Constants.SignatureSpec11NS, Constants._TAG_DERENCODEDKEYVALUE); + for (int i = 0; i < nl.getLength(); i++) { + Element keyInfoElement = (Element) nl.item(i); + keyInfoElement.setIdAttributeNS(null, Constants._ATT_ID, true); + } + } + } + diff --git a/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml b/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml new file mode 100644 index 0000000..f34e3d5 --- /dev/null +++ b/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml @@ -0,0 +1,22 @@ + + + + + MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmDnHagSzfia3N7jOaMSp4VIZjK2lxZgN + X/2z98YLp1XE3cvpP+mOvX3gENWQuX3uoix+2qroZ0BFHzhzf4E7is5Q9+42ZFi5naFk3c/B0Q8A + jtHtWUEZ8VPPBZggz6uJ1ttJS7YDP6XVjaw6SN1bJSD4/lWNIVsh95kuhunbOef6x/kyIbBz9wF4 + S0//G6zPD4GG7/jJ+sDXe+bAgPB1qwhLhrK3N1jGuDZkGGcY/c4b7aba0B0rognwKlygv16GoA/n + zWehxih7clhmMTzP2VWa3Q2GcN8ETe00dz68KtS7GF6W15qftjUvRXEKSoPz86ZsP30jIH1tvIrs + qSh/kwIDAQAB + + + + + + + + + + + +