Skip to content

Commit 21ee364

Browse files
author
Salah Baddou
committed
Java: Add XXE sink model for Woodstox WstxInputFactory
`com.ctc.wstx.stax.WstxInputFactory` overrides `createXMLStreamReader`, `createXMLEventReader` and `setProperty` from `XMLInputFactory`, so the existing `XmlInputFactory` model in `XmlParsers.qll` does not match calls where the static receiver type is `WstxInputFactory` (or its supertype `org.codehaus.stax2.XMLInputFactory2`). Woodstox is vulnerable to XXE in its default configuration, so these missed sinks were false negatives in `java/xxe`. This adds a scoped framework model under `semmle/code/java/frameworks/woodstox/WoodstoxXml.qll` (registered in the `Frameworks` module of `XmlParsers.qll`) that recognises these calls as XXE sinks and treats the factory as safe when both `javax.xml.stream.supportDTD` and `javax.xml.stream.isSupportingExternalEntities` are disabled — mirroring the existing `XMLInputFactory` safe-configuration logic.
1 parent f95ee12 commit 21ee364

File tree

8 files changed

+213
-1
lines changed

8 files changed

+213
-1
lines changed

‎java/ql/lib/semmle/code/java/frameworks/woodstox/WoodstoxXml.qll‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/** Provides definitions related to XML parsing in the Woodstox StAX library. */
2+
overlay[local?]
3+
module;
4+
5+
import java
6+
private import semmle.code.java.security.XmlParsers
7+
8+
/**
9+
* The class `com.ctc.wstx.stax.WstxInputFactory` or its abstract supertype
10+
* `org.codehaus.stax2.XMLInputFactory2`.
11+
*/
12+
private class WstxInputFactory extends RefType {
13+
WstxInputFactory() {
14+
this.hasQualifiedName("com.ctc.wstx.stax", "WstxInputFactory") or
15+
this.hasQualifiedName("org.codehaus.stax2", "XMLInputFactory2")
16+
}
17+
}
18+
19+
/** A call to `WstxInputFactory.createXMLStreamReader`. */
20+
private class WstxInputFactoryStreamReader extends XmlParserCall {
21+
WstxInputFactoryStreamReader() {
22+
exists(Method m |
23+
this.getMethod() = m and
24+
m.getDeclaringType() instanceof WstxInputFactory and
25+
m.hasName("createXMLStreamReader")
26+
)
27+
}
28+
29+
override Expr getSink() {
30+
if this.getMethod().getParameterType(0) instanceof TypeString
31+
then result = this.getArgument(1)
32+
else result = this.getArgument(0)
33+
}
34+
35+
override predicate isSafe() {
36+
SafeWstxInputFactoryFlow::flowsTo(DataFlow::exprNode(this.getQualifier()))
37+
}
38+
}
39+
40+
/** A call to `WstxInputFactory.createXMLEventReader`. */
41+
private class WstxInputFactoryEventReader extends XmlParserCall {
42+
WstxInputFactoryEventReader() {
43+
exists(Method m |
44+
this.getMethod() = m and
45+
m.getDeclaringType() instanceof WstxInputFactory and
46+
m.hasName("createXMLEventReader")
47+
)
48+
}
49+
50+
override Expr getSink() {
51+
if this.getMethod().getParameterType(0) instanceof TypeString
52+
then result = this.getArgument(1)
53+
else result = this.getArgument(0)
54+
}
55+
56+
override predicate isSafe() {
57+
SafeWstxInputFactoryFlow::flowsTo(DataFlow::exprNode(this.getQualifier()))
58+
}
59+
}
60+
61+
/** A `ParserConfig` specific to `WstxInputFactory`. */
62+
private class WstxInputFactoryConfig extends ParserConfig {
63+
WstxInputFactoryConfig() {
64+
exists(Method m |
65+
m = this.getMethod() and
66+
m.getDeclaringType() instanceof WstxInputFactory and
67+
m.hasName("setProperty")
68+
)
69+
}
70+
}
71+
72+
/**
73+
* A safely configured `WstxInputFactory`.
74+
*/
75+
private class SafeWstxInputFactory extends VarAccess {
76+
SafeWstxInputFactory() {
77+
exists(Variable v |
78+
v = this.getVariable() and
79+
exists(WstxInputFactoryConfig config | config.getQualifier() = v.getAnAccess() |
80+
config.disables(configOptionIsSupportingExternalEntities())
81+
) and
82+
exists(WstxInputFactoryConfig config | config.getQualifier() = v.getAnAccess() |
83+
config.disables(configOptionSupportDtd())
84+
)
85+
)
86+
}
87+
}
88+
89+
private predicate safeWstxInputFactoryNode(DataFlow::Node src) {
90+
src.asExpr() instanceof SafeWstxInputFactory
91+
}
92+
93+
private module SafeWstxInputFactoryFlow = DataFlow::SimpleGlobal<safeWstxInputFactoryNode/1>;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private module Frameworks {
1212
private import semmle.code.java.frameworks.javase.Beans
1313
private import semmle.code.java.frameworks.mdht.MdhtXml
1414
private import semmle.code.java.frameworks.rundeck.RundeckXml
15+
private import semmle.code.java.frameworks.woodstox.WoodstoxXml
1516
}
1617

1718
/**

‎java/ql/src/change-notes/2026-04-16-woodstox-xxe.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The queries "Resolving XML external entity in user-controlled data" (`java/xxe`) and "Resolving XML external entity in user-controlled data from local source" (`java/xxe-local`) now recognize sinks in the Woodstox StAX library when `com.ctc.wstx.stax.WstxInputFactory` or `org.codehaus.stax2.XMLInputFactory2` are used directly.

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import java.net.Socket;
2+
3+
import javax.xml.stream.XMLInputFactory;
4+
5+
import com.ctc.wstx.stax.WstxInputFactory;
6+
7+
public class WstxInputFactoryTests {
8+
9+
public void unconfiguredFactory(Socket sock) throws Exception {
10+
WstxInputFactory factory = new WstxInputFactory();
11+
factory.createXMLStreamReader(sock.getInputStream()); // $ Alert
12+
factory.createXMLEventReader(sock.getInputStream()); // $ Alert
13+
}
14+
15+
public void safeFactory(Socket sock) throws Exception {
16+
WstxInputFactory factory = new WstxInputFactory();
17+
factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
18+
factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
19+
factory.createXMLStreamReader(sock.getInputStream()); // safe
20+
factory.createXMLEventReader(sock.getInputStream()); // safe
21+
}
22+
23+
public void safeFactoryStringProperties(Socket sock) throws Exception {
24+
WstxInputFactory factory = new WstxInputFactory();
25+
factory.setProperty("javax.xml.stream.supportDTD", false);
26+
factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
27+
factory.createXMLStreamReader(sock.getInputStream()); // safe
28+
factory.createXMLEventReader(sock.getInputStream()); // safe
29+
}
30+
31+
public void misConfiguredFactory(Socket sock) throws Exception {
32+
WstxInputFactory factory = new WstxInputFactory();
33+
factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
34+
factory.createXMLStreamReader(sock.getInputStream()); // $ Alert
35+
factory.createXMLEventReader(sock.getInputStream()); // $ Alert
36+
}
37+
38+
public void misConfiguredFactory2(Socket sock) throws Exception {
39+
WstxInputFactory factory = new WstxInputFactory();
40+
factory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
41+
factory.createXMLStreamReader(sock.getInputStream()); // $ Alert
42+
factory.createXMLEventReader(sock.getInputStream()); // $ Alert
43+
}
44+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,12 @@
8989
| TransformerTests.java:141:21:141:73 | new SAXSource(...) | TransformerTests.java:141:51:141:71 | getInputStream(...) : InputStream | TransformerTests.java:141:21:141:73 | new SAXSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | TransformerTests.java:141:51:141:71 | getInputStream(...) | user-provided value |
9090
| UnmarshallerTests.java:29:18:29:38 | getInputStream(...) | UnmarshallerTests.java:29:18:29:38 | getInputStream(...) | UnmarshallerTests.java:29:18:29:38 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | UnmarshallerTests.java:29:18:29:38 | getInputStream(...) | user-provided value |
9191
| ValidatorTests.java:22:28:22:33 | source | ValidatorTests.java:17:49:17:72 | getInputStream(...) : ServletInputStream | ValidatorTests.java:22:28:22:33 | source | XML parsing depends on a $@ without guarding against external entity expansion. | ValidatorTests.java:17:49:17:72 | getInputStream(...) | user-provided value |
92+
| WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | user-provided value |
93+
| WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | user-provided value |
94+
| WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | user-provided value |
95+
| WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | user-provided value |
96+
| WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | user-provided value |
97+
| WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | XML parsing depends on a $@ without guarding against external entity expansion. | WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | user-provided value |
9298
| XMLDecoderTests.java:18:9:18:18 | xmlDecoder | XMLDecoderTests.java:16:49:16:72 | getInputStream(...) : ServletInputStream | XMLDecoderTests.java:18:9:18:18 | xmlDecoder | XML parsing depends on a $@ without guarding against external entity expansion. | XMLDecoderTests.java:16:49:16:72 | getInputStream(...) | user-provided value |
9399
| XMLReaderTests.java:16:18:16:55 | new InputSource(...) | XMLReaderTests.java:16:34:16:54 | getInputStream(...) : InputStream | XMLReaderTests.java:16:18:16:55 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XMLReaderTests.java:16:34:16:54 | getInputStream(...) | user-provided value |
94100
| XMLReaderTests.java:56:18:56:55 | new InputSource(...) | XMLReaderTests.java:56:34:56:54 | getInputStream(...) : InputStream | XMLReaderTests.java:56:18:56:55 | new InputSource(...) | XML parsing depends on a $@ without guarding against external entity expansion. | XMLReaderTests.java:56:34:56:54 | getInputStream(...) | user-provided value |
@@ -390,6 +396,12 @@ nodes
390396
| ValidatorTests.java:21:31:21:66 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource |
391397
| ValidatorTests.java:21:48:21:65 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream |
392398
| ValidatorTests.java:22:28:22:33 | source | semmle.label | source |
399+
| WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | semmle.label | getInputStream(...) |
400+
| WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | semmle.label | getInputStream(...) |
401+
| WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | semmle.label | getInputStream(...) |
402+
| WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | semmle.label | getInputStream(...) |
403+
| WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | semmle.label | getInputStream(...) |
404+
| WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | semmle.label | getInputStream(...) |
393405
| XMLDecoderTests.java:16:49:16:72 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream |
394406
| XMLDecoderTests.java:17:33:17:66 | new XMLDecoder(...) : XMLDecoder | semmle.label | new XMLDecoder(...) : XMLDecoder |
395407
| XMLDecoderTests.java:17:48:17:65 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream |

‎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/
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

‎java/ql/test/stubs/woodstox-core-6.4.0/com/ctc/wstx/stax/WstxInputFactory.java‎

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

‎java/ql/test/stubs/woodstox-core-6.4.0/org/codehaus/stax2/XMLInputFactory2.java‎

Lines changed: 9 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)