mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
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.
This commit is contained in:
committed by
Salah Baddou
parent
29b07d5d07
commit
f5131f9bc6
@@ -0,0 +1,93 @@
|
||||
/** 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<safeWstxInputFactoryNode/1>;
|
||||
@@ -12,6 +12,7 @@ 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
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user