From fb2d53e72a5fc189472a1be7163c5960c6c9cc29 Mon Sep 17 00:00:00 2001 From: Salah Baddou Date: Fri, 17 Apr 2026 18:32:24 +0400 Subject: [PATCH] Address review: inline Woodstox into XmlParsers, move changelog to lib --- .../change-notes/2026-04-16-woodstox-xxe.md | 0 .../java/frameworks/woodstox/WoodstoxXml.qll | 93 ------------------- .../semmle/code/java/security/XmlParsers.qll | 32 ++++++- 3 files changed, 27 insertions(+), 98 deletions(-) rename java/ql/{src => lib}/change-notes/2026-04-16-woodstox-xxe.md (100%) delete mode 100644 java/ql/lib/semmle/code/java/frameworks/woodstox/WoodstoxXml.qll diff --git a/java/ql/src/change-notes/2026-04-16-woodstox-xxe.md b/java/ql/lib/change-notes/2026-04-16-woodstox-xxe.md similarity index 100% rename from java/ql/src/change-notes/2026-04-16-woodstox-xxe.md rename to java/ql/lib/change-notes/2026-04-16-woodstox-xxe.md diff --git a/java/ql/lib/semmle/code/java/frameworks/woodstox/WoodstoxXml.qll b/java/ql/lib/semmle/code/java/frameworks/woodstox/WoodstoxXml.qll deleted file mode 100644 index b5964aba3ba..00000000000 --- a/java/ql/lib/semmle/code/java/frameworks/woodstox/WoodstoxXml.qll +++ /dev/null @@ -1,93 +0,0 @@ -/** Provides definitions related to XML parsing in the Woodstox StAX library. */ -overlay[local?] -module; - -import java -private import semmle.code.java.security.XmlParsers - -/** - * The class `com.ctc.wstx.stax.WstxInputFactory` or its abstract supertype - * `org.codehaus.stax2.XMLInputFactory2`. - */ -private class WstxInputFactory extends RefType { - WstxInputFactory() { - this.hasQualifiedName("com.ctc.wstx.stax", "WstxInputFactory") or - this.hasQualifiedName("org.codehaus.stax2", "XMLInputFactory2") - } -} - -/** A call to `WstxInputFactory.createXMLStreamReader`. */ -private class WstxInputFactoryStreamReader extends XmlParserCall { - WstxInputFactoryStreamReader() { - exists(Method m | - this.getMethod() = m and - m.getDeclaringType() instanceof WstxInputFactory and - m.hasName("createXMLStreamReader") - ) - } - - override Expr getSink() { - if this.getMethod().getParameterType(0) instanceof TypeString - then result = this.getArgument(1) - else result = this.getArgument(0) - } - - override predicate isSafe() { - SafeWstxInputFactoryFlow::flowsTo(DataFlow::exprNode(this.getQualifier())) - } -} - -/** A call to `WstxInputFactory.createXMLEventReader`. */ -private class WstxInputFactoryEventReader extends XmlParserCall { - WstxInputFactoryEventReader() { - exists(Method m | - this.getMethod() = m and - m.getDeclaringType() instanceof WstxInputFactory and - m.hasName("createXMLEventReader") - ) - } - - override Expr getSink() { - if this.getMethod().getParameterType(0) instanceof TypeString - then result = this.getArgument(1) - else result = this.getArgument(0) - } - - override predicate isSafe() { - SafeWstxInputFactoryFlow::flowsTo(DataFlow::exprNode(this.getQualifier())) - } -} - -/** A `ParserConfig` specific to `WstxInputFactory`. */ -private class WstxInputFactoryConfig extends ParserConfig { - WstxInputFactoryConfig() { - exists(Method m | - m = this.getMethod() and - m.getDeclaringType() instanceof WstxInputFactory and - m.hasName("setProperty") - ) - } -} - -/** - * A safely configured `WstxInputFactory`. - */ -private class SafeWstxInputFactory extends VarAccess { - SafeWstxInputFactory() { - exists(Variable v | - v = this.getVariable() and - exists(WstxInputFactoryConfig config | config.getQualifier() = v.getAnAccess() | - config.disables(configOptionIsSupportingExternalEntities()) - ) and - exists(WstxInputFactoryConfig config | config.getQualifier() = v.getAnAccess() | - config.disables(configOptionSupportDtd()) - ) - ) - } -} - -private predicate safeWstxInputFactoryNode(DataFlow::Node src) { - src.asExpr() instanceof SafeWstxInputFactory -} - -private module SafeWstxInputFactoryFlow = DataFlow::SimpleGlobal; diff --git a/java/ql/lib/semmle/code/java/security/XmlParsers.qll b/java/ql/lib/semmle/code/java/security/XmlParsers.qll index a910e34c8e3..602076996a7 100644 --- a/java/ql/lib/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/lib/semmle/code/java/security/XmlParsers.qll @@ -12,7 +12,6 @@ private module Frameworks { private import semmle.code.java.frameworks.javase.Beans private import semmle.code.java.frameworks.mdht.MdhtXml private import semmle.code.java.frameworks.rundeck.RundeckXml - private import semmle.code.java.frameworks.woodstox.WoodstoxXml } /** @@ -180,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") ) } @@ -213,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") ) } @@ -236,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") ) }