Skip to content

Commit dc16cbe

Browse files
ppkarwaszppkarwasz
committed
Java: model org.apache.commons.xml XmlFactories as safe XXE sources
Recognize the hardened JAXP factories returned by `org.apache.commons.xml.XmlFactories` (Apache Commons XML) as safely configured, so parsers created from them are no longer reported by the XXE query (CWE-611). Adds an extensible `SafeXmlFactorySource` class to XmlParsers.qll and wires it into the five existing safe-factory flow sources (DocumentBuilderFactory, SAXParserFactory, XMLInputFactory, TransformerFactory, SchemaFactory). `newXPathFactory` is matched for completeness but has no XXE safety chain to feed. The framework model lives in the existing frameworks/apache/CommonsXml.qll. Includes a test stub, safe-case tests, and a change note. Assisted-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1c0ae0f commit dc16cbe

6 files changed

Lines changed: 148 additions & 1 deletion

File tree

‎java/ql/lib/change-notes/2026-08-02-apache-commons-xml-factories.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: feature
3+
---
4+
* Factories returned by the Apache Commons XML (`org.apache.commons.xml.XmlFactories`) hardening library are now recognized as safely configured by the XXE query.
5+
* 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‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,25 @@ private module SafeDigesterFlowConfig implements DataFlow::ConfigSig {
9090
}
9191

9292
private module SafeDigesterFlow = DataFlow::Global<SafeDigesterFlowConfig>;
93+
94+
/**
95+
* A call to one of the `org.apache.commons.xml.XmlFactories.newXxxFactory()` methods
96+
* of the Apache Commons XML library.
97+
*
98+
* Every such method returns a fresh JAXP factory that has already been hardened against
99+
* XML external entity (XXE) attacks, so any parser created from it is treated as safe.
100+
*
101+
* `newXPathFactory` is matched for completeness, but the XXE model has no `XPathFactory`
102+
* safety chain (the XXE sink for XPath is the document being evaluated, not the factory),
103+
* so it currently has no effect on XXE results.
104+
*/
105+
private class CommonsXmlSafeXmlFactory extends SafeXmlFactorySource, MethodCall {
106+
CommonsXmlSafeXmlFactory() {
107+
this.getMethod().getDeclaringType().hasQualifiedName("org.apache.commons.xml", "XmlFactories") and
108+
this.getMethod()
109+
.hasName([
110+
"newDocumentBuilderFactory", "newSAXParserFactory", "newXMLInputFactory",
111+
"newTransformerFactory", "newSchemaFactory", "newXPathFactory"
112+
])
113+
}
114+
}

‎java/ql/lib/semmle/code/java/security/XmlParsers.qll‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@ abstract class ParserConfig extends MethodCall {
5656
}
5757
}
5858

59+
/**
60+
* An expression that evaluates to a JAXP parser factory (such as a
61+
* `DocumentBuilderFactory` or `SAXParserFactory`) that has already been hardened
62+
* against XML external entity (XXE) attacks, for example by a helper library.
63+
*
64+
* Extend this class to model additional sources of pre-hardened JAXP factories.
65+
*/
66+
abstract class SafeXmlFactorySource extends Expr { }
67+
5968
/*
6069
* https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#jaxp-documentbuilderfactory-saxparserfactory-and-dom4j
6170
*/
@@ -156,6 +165,8 @@ private class DocumentBuilderConstruction extends MethodCall {
156165

157166
private predicate safeDocumentBuilderFactoryNode(DataFlow::Node src) {
158167
src.asExpr() instanceof SafeDocumentBuilderFactory
168+
or
169+
src.asExpr().(SafeXmlFactorySource).getType() instanceof DocumentBuilderFactory
159170
}
160171

161172
private module SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlow =
@@ -219,6 +230,8 @@ class XmlInputFactoryStreamReader extends XmlParserCall {
219230

220231
private predicate safeXmlInputFactoryNode(DataFlow::Node src) {
221232
src.asExpr() instanceof SafeXmlInputFactory
233+
or
234+
src.asExpr().(SafeXmlFactorySource).getType() instanceof XmlInputFactory
222235
}
223236

224237
private module SafeXmlInputFactoryToXmlInputFactoryReaderFlow =
@@ -456,6 +469,8 @@ class SafeSaxParserFactory extends VarAccess {
456469

457470
private predicate safeSaxParserFactoryNode(DataFlow::Node src) {
458471
src.asExpr() instanceof SafeSaxParserFactory
472+
or
473+
src.asExpr().(SafeXmlFactorySource).getType() instanceof SaxParserFactory
459474
}
460475

461476
private module SafeSaxParserFactoryToNewSaxParserFlow =
@@ -831,6 +846,8 @@ class TransformerFactoryConfig extends TransformerConfig {
831846

832847
private predicate safeTransformerFactoryNode(DataFlow::Node src) {
833848
src.asExpr() instanceof SafeTransformerFactory
849+
or
850+
src.asExpr().(SafeXmlFactorySource).getType() instanceof TransformerFactory
834851
}
835852

836853
private module SafeTransformerFactoryFlow = DataFlow::SimpleGlobal<safeTransformerFactoryNode/1>;
@@ -920,6 +937,8 @@ class SchemaFactoryNewSchema extends XmlParserCall {
920937

921938
private predicate safeSchemaFactoryNode(DataFlow::Node src) {
922939
src.asExpr() instanceof SafeSchemaFactory
940+
or
941+
src.asExpr().(SafeXmlFactorySource).getType() instanceof SchemaFactory
923942
}
924943

925944
private module SafeSchemaFactoryToSchemaFactoryNewSchemaFlow =

‎java/ql/test/query-tests/security/CWE-611/XmlFactoriesTests.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import java.net.Socket;
2+
3+
import javax.xml.parsers.DocumentBuilder;
4+
import javax.xml.parsers.DocumentBuilderFactory;
5+
import javax.xml.parsers.SAXParser;
6+
import javax.xml.parsers.SAXParserFactory;
7+
import javax.xml.stream.XMLInputFactory;
8+
import javax.xml.transform.Transformer;
9+
import javax.xml.transform.TransformerFactory;
10+
import javax.xml.transform.stream.StreamSource;
11+
import javax.xml.validation.Schema;
12+
import javax.xml.validation.SchemaFactory;
13+
14+
import org.xml.sax.XMLReader;
15+
import org.xml.sax.helpers.DefaultHandler;
16+
17+
import org.apache.commons.xml.XmlFactories;
18+
19+
// Every factory returned by `org.apache.commons.xml.XmlFactories` is already hardened against
20+
// XXE, so the parsers created from them must not be reported.
21+
public class XmlFactoriesTests {
22+
23+
public void hardenedDocumentBuilder(Socket sock) throws Exception {
24+
DocumentBuilderFactory factory = XmlFactories.newDocumentBuilderFactory();
25+
DocumentBuilder builder = factory.newDocumentBuilder();
26+
builder.parse(sock.getInputStream()); // safe
27+
}
28+
29+
public void hardenedDocumentBuilderChained(Socket sock) throws Exception {
30+
XmlFactories.newDocumentBuilderFactory().newDocumentBuilder().parse(sock.getInputStream()); // safe
31+
}
32+
33+
public void hardenedSaxParser(Socket sock) throws Exception {
34+
SAXParserFactory factory = XmlFactories.newSAXParserFactory();
35+
SAXParser parser = factory.newSAXParser();
36+
parser.parse(sock.getInputStream(), new DefaultHandler()); // safe
37+
}
38+
39+
public void hardenedSaxParserXmlReader(Socket sock) throws Exception {
40+
SAXParser parser = XmlFactories.newSAXParserFactory().newSAXParser();
41+
XMLReader reader = parser.getXMLReader();
42+
reader.parse(new org.xml.sax.InputSource(sock.getInputStream())); // safe
43+
}
44+
45+
public void hardenedXmlInputFactory(Socket sock) throws Exception {
46+
XMLInputFactory factory = XmlFactories.newXMLInputFactory();
47+
factory.createXMLStreamReader(sock.getInputStream()); // safe
48+
factory.createXMLEventReader(sock.getInputStream()); // safe
49+
}
50+
51+
public void hardenedTransformer(Socket sock) throws Exception {
52+
TransformerFactory tf = XmlFactories.newTransformerFactory();
53+
Transformer transformer = tf.newTransformer();
54+
transformer.transform(new StreamSource(sock.getInputStream()), null); // safe
55+
tf.newTransformer(new StreamSource(sock.getInputStream())); // safe
56+
}
57+
58+
public void hardenedSchema(Socket sock) throws Exception {
59+
SchemaFactory factory = XmlFactories.newSchemaFactory();
60+
Schema schema = factory.newSchema(new StreamSource(sock.getInputStream())); // safe
61+
}
62+
}

‎java/ql/test/query-tests/security/CWE-611/options‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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
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:${testdir}/../../../stubs/apache-commons-xml-0.1.0

‎java/ql/test/stubs/apache-commons-xml-0.1.0/org/apache/commons/xml/XmlFactories.java‎

Lines changed: 39 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)