Skip to content

Commit 2764580

Browse files
owen-mcowen-mc
authored
Merge pull request #21718 from chmodxxx/java/woodstox-xxe
Java: Add XXE sink model for Woodstox WstxInputFactory
2 parents 29b07d5 + fb2d53e commit 2764580

File tree

7 files changed

+146
-5
lines changed

7 files changed

+146
-5
lines changed

‎java/ql/lib/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/lib/semmle/code/java/security/XmlParsers.qll‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,12 +179,29 @@ class XmlInputFactory extends RefType {
179179
XmlInputFactory() { this.hasQualifiedName(javaxOrJakarta() + ".xml.stream", "XMLInputFactory") }
180180
}
181181

182-
/** A call to `XMLInputFactory.createXMLStreamReader`. */
182+
/**
183+
* The class `com.ctc.wstx.stax.WstxInputFactory` or its abstract supertype
184+
* `org.codehaus.stax2.XMLInputFactory2` from the Woodstox StAX library.
185+
*/
186+
class WstxInputFactory extends RefType {
187+
WstxInputFactory() {
188+
this.hasQualifiedName("com.ctc.wstx.stax", "WstxInputFactory") or
189+
this.hasQualifiedName("org.codehaus.stax2", "XMLInputFactory2")
190+
}
191+
}
192+
193+
/**
194+
* A call to `XMLInputFactory.createXMLStreamReader` or the equivalent method on the
195+
* Woodstox `WstxInputFactory`.
196+
*/
183197
class XmlInputFactoryStreamReader extends XmlParserCall {
184198
XmlInputFactoryStreamReader() {
185199
exists(Method m |
186200
this.getMethod() = m and
187-
m.getDeclaringType() instanceof XmlInputFactory and
201+
(
202+
m.getDeclaringType() instanceof XmlInputFactory or
203+
m.getDeclaringType() instanceof WstxInputFactory
204+
) and
188205
m.hasName("createXMLStreamReader")
189206
)
190207
}
@@ -212,7 +229,10 @@ class XmlInputFactoryEventReader extends XmlParserCall {
212229
XmlInputFactoryEventReader() {
213230
exists(Method m |
214231
this.getMethod() = m and
215-
m.getDeclaringType() instanceof XmlInputFactory and
232+
(
233+
m.getDeclaringType() instanceof XmlInputFactory or
234+
m.getDeclaringType() instanceof WstxInputFactory
235+
) and
216236
m.hasName("createXMLEventReader")
217237
)
218238
}
@@ -235,7 +255,10 @@ class XmlInputFactoryConfig extends ParserConfig {
235255
XmlInputFactoryConfig() {
236256
exists(Method m |
237257
m = this.getMethod() and
238-
m.getDeclaringType() instanceof XmlInputFactory and
258+
(
259+
m.getDeclaringType() instanceof XmlInputFactory or
260+
m.getDeclaringType() instanceof WstxInputFactory
261+
) and
239262
m.hasName("setProperty")
240263
)
241264
}

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