Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2764580

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

7 files changed

+146-5Lines changed: 146 additions & 5 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file
+4Lines changed: 4 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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.
Collapse file

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

Copy file name to clipboardExpand all lines: java/ql/lib/semmle/code/java/security/XmlParsers.qll
+27-4Lines 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
}
Collapse file
+44Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: java/ql/test/query-tests/security/CWE-611/XXE.expected
+12Lines 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 |
Collapse file
+1-1Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: java/ql/test/stubs/woodstox-core-6.4.0/com/ctc/wstx/stax/WstxInputFactory.java
+49Lines changed: 49 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Collapse file

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

Copy file name to clipboardExpand all lines: java/ql/test/stubs/woodstox-core-6.4.0/org/codehaus/stax2/XMLInputFactory2.java
+9Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.