diff --git a/java/ql/lib/change-notes/2026-04-16-woodstox-xxe.md b/java/ql/lib/change-notes/2026-04-16-woodstox-xxe.md new file mode 100644 index 000000000000..891fc489e464 --- /dev/null +++ b/java/ql/lib/change-notes/2026-04-16-woodstox-xxe.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* 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. diff --git a/java/ql/lib/semmle/code/java/security/XmlParsers.qll b/java/ql/lib/semmle/code/java/security/XmlParsers.qll index bd1520034eb9..602076996a77 100644 --- a/java/ql/lib/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/lib/semmle/code/java/security/XmlParsers.qll @@ -179,12 +179,29 @@ class XmlInputFactory extends RefType { XmlInputFactory() { this.hasQualifiedName(javaxOrJakarta() + ".xml.stream", "XMLInputFactory") } } -/** A call to `XMLInputFactory.createXMLStreamReader`. */ +/** + * The class `com.ctc.wstx.stax.WstxInputFactory` or its abstract supertype + * `org.codehaus.stax2.XMLInputFactory2` from the Woodstox StAX library. + */ +class WstxInputFactory extends RefType { + WstxInputFactory() { + this.hasQualifiedName("com.ctc.wstx.stax", "WstxInputFactory") or + this.hasQualifiedName("org.codehaus.stax2", "XMLInputFactory2") + } +} + +/** + * A call to `XMLInputFactory.createXMLStreamReader` or the equivalent method on the + * Woodstox `WstxInputFactory`. + */ class XmlInputFactoryStreamReader extends XmlParserCall { XmlInputFactoryStreamReader() { exists(Method m | this.getMethod() = m and - m.getDeclaringType() instanceof XmlInputFactory and + ( + m.getDeclaringType() instanceof XmlInputFactory or + m.getDeclaringType() instanceof WstxInputFactory + ) and m.hasName("createXMLStreamReader") ) } @@ -212,7 +229,10 @@ class XmlInputFactoryEventReader extends XmlParserCall { XmlInputFactoryEventReader() { exists(Method m | this.getMethod() = m and - m.getDeclaringType() instanceof XmlInputFactory and + ( + m.getDeclaringType() instanceof XmlInputFactory or + m.getDeclaringType() instanceof WstxInputFactory + ) and m.hasName("createXMLEventReader") ) } @@ -235,7 +255,10 @@ class XmlInputFactoryConfig extends ParserConfig { XmlInputFactoryConfig() { exists(Method m | m = this.getMethod() and - m.getDeclaringType() instanceof XmlInputFactory and + ( + m.getDeclaringType() instanceof XmlInputFactory or + m.getDeclaringType() instanceof WstxInputFactory + ) and m.hasName("setProperty") ) } diff --git a/java/ql/test/query-tests/security/CWE-611/WstxInputFactoryTests.java b/java/ql/test/query-tests/security/CWE-611/WstxInputFactoryTests.java new file mode 100644 index 000000000000..b64ec54f5b78 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-611/WstxInputFactoryTests.java @@ -0,0 +1,44 @@ +import java.net.Socket; + +import javax.xml.stream.XMLInputFactory; + +import com.ctc.wstx.stax.WstxInputFactory; + +public class WstxInputFactoryTests { + + public void unconfiguredFactory(Socket sock) throws Exception { + WstxInputFactory factory = new WstxInputFactory(); + factory.createXMLStreamReader(sock.getInputStream()); // $ Alert + factory.createXMLEventReader(sock.getInputStream()); // $ Alert + } + + public void safeFactory(Socket sock) throws Exception { + WstxInputFactory factory = new WstxInputFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + factory.createXMLStreamReader(sock.getInputStream()); // safe + factory.createXMLEventReader(sock.getInputStream()); // safe + } + + public void safeFactoryStringProperties(Socket sock) throws Exception { + WstxInputFactory factory = new WstxInputFactory(); + factory.setProperty("javax.xml.stream.supportDTD", false); + factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false); + factory.createXMLStreamReader(sock.getInputStream()); // safe + factory.createXMLEventReader(sock.getInputStream()); // safe + } + + public void misConfiguredFactory(Socket sock) throws Exception { + WstxInputFactory factory = new WstxInputFactory(); + factory.setProperty("javax.xml.stream.isSupportingExternalEntities", false); + factory.createXMLStreamReader(sock.getInputStream()); // $ Alert + factory.createXMLEventReader(sock.getInputStream()); // $ Alert + } + + public void misConfiguredFactory2(Socket sock) throws Exception { + WstxInputFactory factory = new WstxInputFactory(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.createXMLStreamReader(sock.getInputStream()); // $ Alert + factory.createXMLEventReader(sock.getInputStream()); // $ Alert + } +} diff --git a/java/ql/test/query-tests/security/CWE-611/XXE.expected b/java/ql/test/query-tests/security/CWE-611/XXE.expected index a1d0725321d9..87e033c129a7 100644 --- a/java/ql/test/query-tests/security/CWE-611/XXE.expected +++ b/java/ql/test/query-tests/security/CWE-611/XXE.expected @@ -89,6 +89,12 @@ | 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 | | 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 | | 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | | 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 | | 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 | | 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 | ValidatorTests.java:21:31:21:66 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | | ValidatorTests.java:21:48:21:65 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream | | ValidatorTests.java:22:28:22:33 | source | semmle.label | source | +| WstxInputFactoryTests.java:11:35:11:55 | getInputStream(...) | semmle.label | getInputStream(...) | +| WstxInputFactoryTests.java:12:34:12:54 | getInputStream(...) | semmle.label | getInputStream(...) | +| WstxInputFactoryTests.java:34:35:34:55 | getInputStream(...) | semmle.label | getInputStream(...) | +| WstxInputFactoryTests.java:35:34:35:54 | getInputStream(...) | semmle.label | getInputStream(...) | +| WstxInputFactoryTests.java:41:35:41:55 | getInputStream(...) | semmle.label | getInputStream(...) | +| WstxInputFactoryTests.java:42:34:42:54 | getInputStream(...) | semmle.label | getInputStream(...) | | XMLDecoderTests.java:16:49:16:72 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | | XMLDecoderTests.java:17:33:17:66 | new XMLDecoder(...) : XMLDecoder | semmle.label | new XMLDecoder(...) : XMLDecoder | | XMLDecoderTests.java:17:48:17:65 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream | diff --git a/java/ql/test/query-tests/security/CWE-611/options b/java/ql/test/query-tests/security/CWE-611/options index 1480b49d7168..190e6b2af0c6 100644 --- a/java/ql/test/query-tests/security/CWE-611/options +++ b/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/ +//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 diff --git a/java/ql/test/stubs/woodstox-core-6.4.0/com/ctc/wstx/stax/WstxInputFactory.java b/java/ql/test/stubs/woodstox-core-6.4.0/com/ctc/wstx/stax/WstxInputFactory.java new file mode 100644 index 000000000000..979d76b6e202 --- /dev/null +++ b/java/ql/test/stubs/woodstox-core-6.4.0/com/ctc/wstx/stax/WstxInputFactory.java @@ -0,0 +1,49 @@ +// Generated automatically from com.ctc.wstx.stax.WstxInputFactory for testing purposes + +package com.ctc.wstx.stax; + +import java.io.InputStream; +import java.io.Reader; +import javax.xml.stream.EventFilter; +import javax.xml.stream.StreamFilter; +import javax.xml.stream.XMLEventReader; +import javax.xml.stream.XMLReporter; +import javax.xml.stream.XMLResolver; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.stream.util.XMLEventAllocator; +import javax.xml.transform.Source; +import org.codehaus.stax2.XMLInputFactory2; + +public class WstxInputFactory extends XMLInputFactory2 { + public WstxInputFactory() {} + + public XMLStreamReader createXMLStreamReader(InputStream in) throws XMLStreamException { return null; } + public XMLStreamReader createXMLStreamReader(InputStream in, String enc) throws XMLStreamException { return null; } + public XMLStreamReader createXMLStreamReader(Reader r) throws XMLStreamException { return null; } + public XMLStreamReader createXMLStreamReader(Source src) throws XMLStreamException { return null; } + public XMLStreamReader createXMLStreamReader(String systemId, InputStream in) throws XMLStreamException { return null; } + public XMLStreamReader createXMLStreamReader(String systemId, Reader r) throws XMLStreamException { return null; } + + public XMLEventReader createXMLEventReader(InputStream in) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(InputStream in, String enc) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(Reader r) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(Source src) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(String systemId, InputStream in) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(String systemId, Reader r) throws XMLStreamException { return null; } + public XMLEventReader createXMLEventReader(XMLStreamReader sr) throws XMLStreamException { return null; } + + public XMLStreamReader createFilteredReader(XMLStreamReader reader, StreamFilter filter) { return null; } + public XMLEventReader createFilteredReader(XMLEventReader reader, EventFilter filter) { return null; } + + public void setProperty(String name, Object value) {} + public Object getProperty(String name) { return null; } + public boolean isPropertySupported(String name) { return false; } + + public XMLResolver getXMLResolver() { return null; } + public void setXMLResolver(XMLResolver r) {} + public XMLReporter getXMLReporter() { return null; } + public void setXMLReporter(XMLReporter r) {} + public XMLEventAllocator getEventAllocator() { return null; } + public void setEventAllocator(XMLEventAllocator a) {} +} diff --git a/java/ql/test/stubs/woodstox-core-6.4.0/org/codehaus/stax2/XMLInputFactory2.java b/java/ql/test/stubs/woodstox-core-6.4.0/org/codehaus/stax2/XMLInputFactory2.java new file mode 100644 index 000000000000..6b94d00d03ac --- /dev/null +++ b/java/ql/test/stubs/woodstox-core-6.4.0/org/codehaus/stax2/XMLInputFactory2.java @@ -0,0 +1,9 @@ +// Generated automatically from org.codehaus.stax2.XMLInputFactory2 for testing purposes + +package org.codehaus.stax2; + +import javax.xml.stream.XMLInputFactory; + +public abstract class XMLInputFactory2 extends XMLInputFactory { + protected XMLInputFactory2() {} +}