Merge pull request #22269 from ppkarwasz/feat/xml-commons

Java: model org.apache.commons.xml XmlFactories as safe XXE sources

Author: owen-mc

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.

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<SafeDigesterFlowConfig>;
+
+/**
+ * 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"
+          ])
+  }
+}

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<safeTransformerFactoryNode/1>;
@@ -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 =

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
+  }
+}

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