diff --git a/java/ql/lib/change-notes/2026-08-02-apache-commons-secure-xml.md b/java/ql/lib/change-notes/2026-08-02-apache-commons-secure-xml.md new file mode 100644 index 000000000000..898255b12458 --- /dev/null +++ b/java/ql/lib/change-notes/2026-08-02-apache-commons-secure-xml.md @@ -0,0 +1,5 @@ +--- +category: feature +--- +* Factories returned by the Apache Commons Secure XML (`org.apache.commons.xml.secure`) hardening library's `SecureDocumentBuilderFactory`, `SecureSAXParserFactory`, `SecureXMLInputFactory`, `SecureTransformerFactory` and `SecureSchemaFactory` classes are now recognized as safely configured by the XXE query. +* A new extensible class `SafeXmlFactorySource` was added to `semmle.code.java.security.XmlParsers` for modeling sources of pre-hardened JAXP factories. diff --git a/java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll b/java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll index 163bd773dad0..fa570b0b61ae 100644 --- a/java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll +++ b/java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll @@ -90,3 +90,33 @@ private module SafeDigesterFlowConfig implements DataFlow::ConfigSig { } private module SafeDigesterFlow = DataFlow::Global; + +/** + * A call to one of the static factory methods of the `org.apache.commons.xml.secure` + * `SecureXxxFactory` classes of the Apache Commons Secure XML library. + * + * These methods mirror the JAXP factory entry points (`newInstance`, `newDefaultInstance`, + * `newNSInstance`, `newFactory`, ...) and every one of them returns a fresh JAXP factory + * that has already been hardened against XML external entity (XXE) attacks, so any parser + * created from it is treated as safe. + * + * `SecureXPathFactory` is matched for completeness, but the XXE model has no `XPathFactory` + * safety chain (the XXE sink for XPath is the document being evaluated, not the factory), + * so it currently has no effect on XXE results. + */ +private class CommonsSecureXmlFactory extends SafeXmlFactorySource, MethodCall { + CommonsSecureXmlFactory() { + this.getMethod() + .getDeclaringType() + .hasQualifiedName("org.apache.commons.xml.secure", + [ + "SecureDocumentBuilderFactory", "SecureSAXParserFactory", "SecureXMLInputFactory", + "SecureTransformerFactory", "SecureSchemaFactory", "SecureXPathFactory" + ]) and + this.getMethod() + .hasName([ + "newDefaultInstance", "newDefaultNSInstance", "newInstance", "newNSInstance", + "newDefaultFactory", "newFactory" + ]) + } +} diff --git a/java/ql/lib/semmle/code/java/security/XmlParsers.qll b/java/ql/lib/semmle/code/java/security/XmlParsers.qll index 602076996a77..4a30ded19e3a 100644 --- a/java/ql/lib/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/lib/semmle/code/java/security/XmlParsers.qll @@ -56,6 +56,15 @@ abstract class ParserConfig extends MethodCall { } } +/** + * An expression that evaluates to a JAXP parser factory (such as a + * `DocumentBuilderFactory` or `SAXParserFactory`) that has already been hardened + * against XML external entity (XXE) attacks, for example by a helper library. + * + * Extend this class to model additional sources of pre-hardened JAXP factories. + */ +abstract class SafeXmlFactorySource extends Expr { } + /* * https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j */ @@ -156,6 +165,8 @@ private class DocumentBuilderConstruction extends MethodCall { private predicate safeDocumentBuilderFactoryNode(DataFlow::Node src) { src.asExpr() instanceof SafeDocumentBuilderFactory + or + src.asExpr().(SafeXmlFactorySource).getType() instanceof DocumentBuilderFactory } private module SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlow = @@ -219,6 +230,8 @@ class XmlInputFactoryStreamReader extends XmlParserCall { private predicate safeXmlInputFactoryNode(DataFlow::Node src) { src.asExpr() instanceof SafeXmlInputFactory + or + src.asExpr().(SafeXmlFactorySource).getType() instanceof XmlInputFactory } private module SafeXmlInputFactoryToXmlInputFactoryReaderFlow = @@ -456,6 +469,8 @@ class SafeSaxParserFactory extends VarAccess { private predicate safeSaxParserFactoryNode(DataFlow::Node src) { src.asExpr() instanceof SafeSaxParserFactory + or + src.asExpr().(SafeXmlFactorySource).getType() instanceof SaxParserFactory } private module SafeSaxParserFactoryToNewSaxParserFlow = @@ -831,6 +846,8 @@ class TransformerFactoryConfig extends TransformerConfig { private predicate safeTransformerFactoryNode(DataFlow::Node src) { src.asExpr() instanceof SafeTransformerFactory + or + src.asExpr().(SafeXmlFactorySource).getType() instanceof TransformerFactory } private module SafeTransformerFactoryFlow = DataFlow::SimpleGlobal; @@ -920,6 +937,8 @@ class SchemaFactoryNewSchema extends XmlParserCall { private predicate safeSchemaFactoryNode(DataFlow::Node src) { src.asExpr() instanceof SafeSchemaFactory + or + src.asExpr().(SafeXmlFactorySource).getType() instanceof SchemaFactory } private module SafeSchemaFactoryToSchemaFactoryNewSchemaFlow = diff --git a/java/ql/test/query-tests/security/CWE-611/SecureXmlFactoriesTests.java b/java/ql/test/query-tests/security/CWE-611/SecureXmlFactoriesTests.java new file mode 100644 index 000000000000..77c6dccfed73 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-611/SecureXmlFactoriesTests.java @@ -0,0 +1,77 @@ +import java.net.Socket; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.SAXParser; +import javax.xml.parsers.SAXParserFactory; +import javax.xml.stream.XMLInputFactory; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; + +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.DefaultHandler; + +import org.apache.commons.xml.secure.SecureDocumentBuilderFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; +import org.apache.commons.xml.secure.SecureSchemaFactory; +import org.apache.commons.xml.secure.SecureTransformerFactory; +import org.apache.commons.xml.secure.SecureXMLInputFactory; + +// Every factory returned by the `org.apache.commons.xml.secure.SecureXxxFactory` classes is +// already hardened against XXE, so the parsers created from them must not be reported. +public class SecureXmlFactoriesTests { + + public void hardenedDocumentBuilder(Socket sock) throws Exception { + DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newInstance(); + DocumentBuilder builder = factory.newDocumentBuilder(); + builder.parse(sock.getInputStream()); // safe + } + + public void hardenedDocumentBuilderChained(Socket sock) throws Exception { + SecureDocumentBuilderFactory.newDefaultNSInstance().newDocumentBuilder().parse(sock.getInputStream()); // safe + } + + public void hardenedSaxParser(Socket sock) throws Exception { + SAXParserFactory factory = SecureSAXParserFactory.newInstance(); + SAXParser parser = factory.newSAXParser(); + parser.parse(sock.getInputStream(), new DefaultHandler()); // safe + } + + public void hardenedSaxParserXmlReader(Socket sock) throws Exception { + SAXParser parser = SecureSAXParserFactory.newNSInstance().newSAXParser(); + XMLReader reader = parser.getXMLReader(); + reader.parse(new org.xml.sax.InputSource(sock.getInputStream())); // safe + } + + public void hardenedXmlInputFactory(Socket sock) throws Exception { + XMLInputFactory factory = SecureXMLInputFactory.newFactory(); + factory.createXMLStreamReader(sock.getInputStream()); // safe + factory.createXMLEventReader(sock.getInputStream()); // safe + } + + public void hardenedXmlInputFactoryDefault(Socket sock) throws Exception { + XMLInputFactory factory = SecureXMLInputFactory.newDefaultFactory(); + factory.createXMLStreamReader(sock.getInputStream()); // safe + } + + public void hardenedTransformer(Socket sock) throws Exception { + TransformerFactory tf = SecureTransformerFactory.newInstance(); + Transformer transformer = tf.newTransformer(); + transformer.transform(new StreamSource(sock.getInputStream()), null); // safe + tf.newTransformer(new StreamSource(sock.getInputStream())); // safe + } + + public void hardenedTransformerDefault(Socket sock) throws Exception { + TransformerFactory tf = SecureTransformerFactory.newDefaultInstance(); + tf.newTransformer(new StreamSource(sock.getInputStream())); // safe + } + + public void hardenedSchema(Socket sock) throws Exception { + SchemaFactory factory = SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + Schema schema = factory.newSchema(new StreamSource(sock.getInputStream())); // safe + } +} diff --git a/java/ql/test/query-tests/security/CWE-611/options b/java/ql/test/query-tests/security/CWE-611/options index 190e6b2af0c6..c7186f87c849 100644 --- a/java/ql/test/query-tests/security/CWE-611/options +++ b/java/ql/test/query-tests/security/CWE-611/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/jdom-1.1.3:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/simple-xml-2.7.1:${testdir}/../../../stubs/jaxb-api-2.3.1:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/apache-commons-digester3-3.2:${testdir}/../../../stubs/servlet-api-2.4/:${testdir}/../../../stubs/rundeck-api-java-client-13.2:${testdir}/../../../stubs/springframework-5.8.x/:${testdir}/../../../stubs/mdht-1.2.0/:${testdir}/../../../stubs/woodstox-core-6.4.0 +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/jdom-1.1.3:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/simple-xml-2.7.1:${testdir}/../../../stubs/jaxb-api-2.3.1:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/apache-commons-digester3-3.2:${testdir}/../../../stubs/servlet-api-2.4/:${testdir}/../../../stubs/rundeck-api-java-client-13.2:${testdir}/../../../stubs/springframework-5.8.x/:${testdir}/../../../stubs/mdht-1.2.0/:${testdir}/../../../stubs/woodstox-core-6.4.0:${testdir}/../../../stubs/apache-commons-secure-xml-1.0.0 diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java new file mode 100644 index 000000000000..22011e425aa2 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java @@ -0,0 +1,34 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureDocumentBuilderFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.parsers.DocumentBuilderFactory; + +public final class SecureDocumentBuilderFactory { + + public static DocumentBuilderFactory newDefaultInstance() { + return null; + } + + public static DocumentBuilderFactory newDefaultNSInstance() { + return null; + } + + public static DocumentBuilderFactory newInstance() { + return null; + } + + public static DocumentBuilderFactory newInstance(final String factoryClassName, final ClassLoader classLoader) { + return null; + } + + public static DocumentBuilderFactory newNSInstance() { + return null; + } + + public static DocumentBuilderFactory newNSInstance(final String factoryClassName, final ClassLoader classLoader) { + return null; + } + + private SecureDocumentBuilderFactory() {} +} diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSAXParserFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSAXParserFactory.java new file mode 100644 index 000000000000..4a0adcb44617 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSAXParserFactory.java @@ -0,0 +1,34 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureSAXParserFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.parsers.SAXParserFactory; + +public final class SecureSAXParserFactory { + + public static SAXParserFactory newDefaultInstance() { + return null; + } + + public static SAXParserFactory newDefaultNSInstance() { + return null; + } + + public static SAXParserFactory newInstance() { + return null; + } + + public static SAXParserFactory newInstance(final String factoryClassName, final ClassLoader classLoader) { + return null; + } + + public static SAXParserFactory newNSInstance() { + return null; + } + + public static SAXParserFactory newNSInstance(final String factoryClassName, final ClassLoader classLoader) { + return null; + } + + private SecureSAXParserFactory() {} +} diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSchemaFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSchemaFactory.java new file mode 100644 index 000000000000..5c0d5ac258de --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureSchemaFactory.java @@ -0,0 +1,23 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureSchemaFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.validation.SchemaFactory; + +public final class SecureSchemaFactory { + + public static SchemaFactory newDefaultInstance() { + return null; + } + + public static SchemaFactory newInstance(final String schemaLanguage) { + return null; + } + + public static SchemaFactory newInstance(final String schemaLanguage, final String factoryClassName, + final ClassLoader classLoader) { + return null; + } + + private SecureSchemaFactory() {} +} diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureTransformerFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureTransformerFactory.java new file mode 100644 index 000000000000..c51bb648ca10 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureTransformerFactory.java @@ -0,0 +1,22 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureTransformerFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.transform.TransformerFactory; + +public final class SecureTransformerFactory { + + public static TransformerFactory newDefaultInstance() { + return null; + } + + public static TransformerFactory newInstance() { + return null; + } + + public static TransformerFactory newInstance(final String factoryClassName, final ClassLoader classLoader) { + return null; + } + + private SecureTransformerFactory() {} +} diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXMLInputFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXMLInputFactory.java new file mode 100644 index 000000000000..7116d3c14ae4 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXMLInputFactory.java @@ -0,0 +1,26 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureXMLInputFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.stream.XMLInputFactory; + +public final class SecureXMLInputFactory { + + public static XMLInputFactory newDefaultFactory() { + return null; + } + + public static XMLInputFactory newFactory() { + return null; + } + + public static XMLInputFactory newFactory(final String factoryId, final ClassLoader classLoader) { + return null; + } + + public static XMLInputFactory newInstance() { + return null; + } + + private SecureXMLInputFactory() {} +} diff --git a/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXPathFactory.java b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXPathFactory.java new file mode 100644 index 000000000000..b7bc969bf7f7 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-secure-xml-1.0.0/org/apache/commons/xml/secure/SecureXPathFactory.java @@ -0,0 +1,28 @@ +// Minimal stub of org.apache.commons.xml.secure.SecureXPathFactory for testing purposes + +package org.apache.commons.xml.secure; + +import javax.xml.xpath.XPathFactory; +import javax.xml.xpath.XPathFactoryConfigurationException; + +public final class SecureXPathFactory { + + public static XPathFactory newDefaultInstance() { + return null; + } + + public static XPathFactory newInstance() { + return null; + } + + public static XPathFactory newInstance(final String uri) throws XPathFactoryConfigurationException { + return null; + } + + public static XPathFactory newInstance(final String uri, final String factoryClassName, final ClassLoader classLoader) + throws XPathFactoryConfigurationException { + return null; + } + + private SecureXPathFactory() {} +}