hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 02:05:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refactor CWE-502/UnsafeDeserialization

Browse Source
This commit is contained in:
Ed Minnix
2023-03-16 15:41:24 -04:00
parent 434b1b35d8
commit e5f11d00a7
4 changed files with 265 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
java/ql/lib/semmle/code/java/frameworks/JsonIo.qll
View File

@@ -42,8 +42,12 @@ class JsonIoUseMapsSetter extends MethodAccess {
}
}
/** A data flow configuration tracing flow from JsonIo safe settings. */
class SafeJsonIoConfig extends DataFlow2::Configuration {
/**
* DEPRECATED: Use `SafeJsonIoFlow` instead.
*
* A data flow configuration tracing flow from JsonIo safe settings.
*/
deprecated class SafeJsonIoConfig extends DataFlow2::Configuration {
SafeJsonIoConfig() { this = "UnsafeDeserialization::SafeJsonIoConfig" }
override predicate isSource(DataFlow::Node src) {
@@ -65,3 +69,30 @@ class SafeJsonIoConfig extends DataFlow2::Configuration {
)
}
}
/**
* A data flow configuration tracing flow from JsonIo safe settings.
*/
module SafeJsonIoConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
exists(MethodAccess ma |
ma instanceof JsonIoUseMapsSetter and
src.asExpr() = ma.getQualifier()
)
}
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
sink.asExpr() = ma.getArgument(1)
)
or
exists(ClassInstanceExpr cie |
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
sink.asExpr() = cie.getArgument(1)
)
}
}
/** Tracks flow from JsonIo safe settings. */
module SafeJsonIoFlow = DataFlow::Global<SafeJsonIoConfig>;

347
java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll
View File

@@ -35,15 +35,13 @@ private class XmlDecoderReadObjectMethod extends Method {
}
}
private class SafeXStream extends DataFlow2::Configuration {
SafeXStream() { this = "UnsafeDeserialization::SafeXStream" }
override predicate isSource(DataFlow::Node src) {
private module SafeXStreamConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
any(XStreamEnableWhiteListing ma).getQualifier().(VarAccess).getVariable().getAnAccess() =
src.asExpr()
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
sink.asExpr() = ma.getQualifier() and
ma.getMethod() instanceof XStreamReadObjectMethod
@@ -51,33 +49,33 @@ private class SafeXStream extends DataFlow2::Configuration {
}
}
private class SafeKryo extends DataFlow2::Configuration {
SafeKryo() { this = "UnsafeDeserialization::SafeKryo" }
private module SafeXStreamFlow = DataFlow::Global<SafeXStreamConfig>;
override predicate isSource(DataFlow::Node src) {
private module SafeKryoConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
any(KryoEnableWhiteListing ma).getQualifier().(VarAccess).getVariable().getAnAccess() =
src.asExpr()
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
sink.asExpr() = ma.getQualifier() and
ma.getMethod() instanceof KryoReadObjectMethod
)
}
override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
this.stepKryoPoolBuilderFactoryArgToConstructor(node1, node2) or
this.stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(node1, node2) or
this.stepKryoPoolBuilderChainMethod(node1, node2) or
this.stepKryoPoolBorrowMethod(node1, node2)
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
stepKryoPoolBuilderFactoryArgToConstructor(node1, node2) or
stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(node1, node2) or
stepKryoPoolBuilderChainMethod(node1, node2) or
stepKryoPoolBorrowMethod(node1, node2)
}
/**
* Holds when a functional expression is used to create a `KryoPool.Builder`.
* Eg. `new KryoPool.Builder(() -> new Kryo())`
*/
private predicate stepKryoPoolBuilderFactoryArgToConstructor(
additional predicate stepKryoPoolBuilderFactoryArgToConstructor(
DataFlow::Node node1, DataFlow::Node node2
) {
exists(ConstructorCall cc, FunctionalExpr fe |
@@ -92,7 +90,7 @@ private class SafeKryo extends DataFlow2::Configuration {
* Holds when a `KryoPool.run` is called to use a `Kryo` instance.
* Eg. `pool.run(kryo -> ...)`
*/
private predicate stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(
additional predicate stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(
DataFlow::Node node1, DataFlow::Node node2
) {
exists(MethodAccess ma |
@@ -105,7 +103,7 @@ private class SafeKryo extends DataFlow2::Configuration {
/**
* Holds when a `KryoPool.Builder` method is called fluently.
*/
private predicate stepKryoPoolBuilderChainMethod(DataFlow::Node node1, DataFlow::Node node2) {
additional predicate stepKryoPoolBuilderChainMethod(DataFlow::Node node1, DataFlow::Node node2) {
exists(MethodAccess ma |
ma.getMethod() instanceof KryoPoolBuilderMethod and
ma = node2.asExpr() and
@@ -116,7 +114,7 @@ private class SafeKryo extends DataFlow2::Configuration {
/**
* Holds when a `KryoPool.borrow` method is called.
*/
private predicate stepKryoPoolBorrowMethod(DataFlow::Node node1, DataFlow::Node node2) {
additional predicate stepKryoPoolBorrowMethod(DataFlow::Node node1, DataFlow::Node node2) {
exists(MethodAccess ma |
ma.getMethod() =
any(Method m | m.getDeclaringType() instanceof KryoPool and m.hasName("borrow")) and
@@ -126,6 +124,8 @@ private class SafeKryo extends DataFlow2::Configuration {
}
}
private module SafeKryoFlow = DataFlow::Global<SafeKryoConfig>;
/**
* Holds if `ma` is a call that deserializes data from `sink`.
*/
@@ -145,11 +145,11 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
or
m instanceof XStreamReadObjectMethod and
sink = ma.getAnArgument() and
not exists(SafeXStream sxs | sxs.hasFlowToExpr(ma.getQualifier()))
not SafeXStreamFlow::flowToExpr(ma.getQualifier())
or
m instanceof KryoReadObjectMethod and
sink = ma.getAnArgument() and
not exists(SafeKryo sk | sk.hasFlowToExpr(ma.getQualifier()))
not SafeKryoFlow::flowToExpr(ma.getQualifier())
or
m instanceof MethodApacheSerializationUtilsDeserialize and
sink = ma.getArgument(0)
@@ -181,17 +181,17 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
ma.getMethod() instanceof ObjectMapperReadMethod and
sink = ma.getArgument(0) and
(
exists(UnsafeTypeConfig config | config.hasFlowToExpr(ma.getAnArgument()))
UnsafeTypeFlow::flowToExpr(ma.getAnArgument())
or
exists(EnableJacksonDefaultTypingConfig config | config.hasFlowToExpr(ma.getQualifier()))
EnableJacksonDefaultTypingFlow::flowToExpr(ma.getQualifier())
or
hasArgumentWithUnsafeJacksonAnnotation(ma)
) and
not exists(SafeObjectMapperConfig config | config.hasFlowToExpr(ma.getQualifier()))
not SafeObjectMapperFlow::flowToExpr(ma.getQualifier())
or
m instanceof JabsorbUnmarshallMethod and
sink = ma.getArgument(2) and
any(UnsafeTypeConfig config).hasFlowToExpr(ma.getArgument(1))
UnsafeTypeFlow::flowToExpr(ma.getArgument(1))
or
m instanceof JabsorbFromJsonMethod and
sink = ma.getArgument(0)
@@ -200,7 +200,7 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
sink = ma.getArgument(0) and
(
// User controls the target type for deserialization
any(UnsafeTypeConfig c).hasFlowToExpr(ma.getArgument(1))
UnsafeTypeFlow::flowToExpr(ma.getArgument(1))
or
// jodd.json.JsonParser may be configured for unrestricted deserialization to user-specified types
joddJsonParserConfiguredUnsafely(ma.getQualifier())
@@ -211,7 +211,7 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
or
m instanceof GsonDeserializeMethod and
sink = ma.getArgument(0) and
any(UnsafeTypeConfig config).hasFlowToExpr(ma.getArgument(1))
UnsafeTypeFlow::flowToExpr(ma.getArgument(1))
)
}
@@ -223,17 +223,53 @@ class UnsafeDeserializationSink extends DataFlow::ExprNode {
MethodAccess getMethodAccess() { unsafeDeserialization(result, this.getExpr()) }
}
/**
* Tracks flows from remote user input to a deserialization sink.
*/
class UnsafeDeserializationConfig extends TaintTracking::Configuration {
UnsafeDeserializationConfig() { this = "UnsafeDeserializationConfig" }
/** A sanitizer for unsafe deserialization */
private class UnsafeDeserializationSanitizer extends DataFlow::Node {
UnsafeDeserializationSanitizer() {
exists(ClassInstanceExpr cie |
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
cie = this.asExpr() and
SafeJsonIoFlow::flowToExpr(cie.getArgument(1))
)
or
exists(MethodAccess ma |
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
ma.getArgument(0) = this.asExpr() and
SafeJsonIoFlow::flowToExpr(ma.getArgument(1))
)
or
exists(MethodAccess ma |
// Sanitize the input to jodd.json.JsonParser.parse et al whenever it appears
// to be called with an explicit class argument limiting those types that can
// be instantiated during deserialization.
ma.getMethod() instanceof JoddJsonParseMethod and
ma.getArgument(1).getType() instanceof TypeClass and
not ma.getArgument(1) instanceof NullLiteral and
not ma.getArgument(1).getType().getName() = ["Class<Object>", "Class<?>"] and
this.asExpr() = ma.getAnArgument()
)
or
exists(MethodAccess ma |
// Sanitize the input to flexjson.JSONDeserializer.deserialize whenever it appears
// to be called with an explicit class argument limiting those types that can
// be instantiated during deserialization, or if the deserializer has already been
// configured to use a specified root class.
ma.getMethod() instanceof FlexjsonDeserializeMethod and
this.asExpr() = ma.getAnArgument() and
(
ma.getArgument(1).getType() instanceof TypeClass and
not ma.getArgument(1) instanceof NullLiteral and
not ma.getArgument(1).getType().getName() = ["Class<Object>", "Class<?>"]
or
isSafeFlexjsonDeserializer(ma.getQualifier())
)
)
}
}
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
/** Taint step for Unsafe deserialization */
private class UnsafeDeserializationAdditionalTaintStep extends Unit {
predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(ClassInstanceExpr cie |
cie.getArgument(0) = pred.asExpr() and
cie = succ.asExpr() and
@@ -257,49 +293,44 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
or
intentFlowsToParcel(pred, succ)
}
}
/**
* DEPRECATED: Use `UnsafeDeserializationFlow` instead.
*
* Tracks flows from remote user input to a deserialization sink.
*/
deprecated class UnsafeDeserializationConfig extends TaintTracking::Configuration {
UnsafeDeserializationConfig() { this = "UnsafeDeserializationConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
any(UnsafeDeserializationAdditionalTaintStep s).isAdditionalTaintStep(pred, succ)
}
override predicate isSanitizer(DataFlow::Node node) {
exists(ClassInstanceExpr cie |
cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and
cie = node.asExpr() and
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1)))
)
or
exists(MethodAccess ma |
ma.getMethod() instanceof JsonIoJsonToJavaMethod and
ma.getArgument(0) = node.asExpr() and
exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
)
or
exists(MethodAccess ma |
// Sanitize the input to jodd.json.JsonParser.parse et al whenever it appears
// to be called with an explicit class argument limiting those types that can
// be instantiated during deserialization.
ma.getMethod() instanceof JoddJsonParseMethod and
ma.getArgument(1).getType() instanceof TypeClass and
not ma.getArgument(1) instanceof NullLiteral and
not ma.getArgument(1).getType().getName() = ["Class<Object>", "Class<?>"] and
node.asExpr() = ma.getAnArgument()
)
or
exists(MethodAccess ma |
// Sanitize the input to flexjson.JSONDeserializer.deserialize whenever it appears
// to be called with an explicit class argument limiting those types that can
// be instantiated during deserialization, or if the deserializer has already been
// configured to use a specified root class.
ma.getMethod() instanceof FlexjsonDeserializeMethod and
node.asExpr() = ma.getAnArgument() and
(
ma.getArgument(1).getType() instanceof TypeClass and
not ma.getArgument(1) instanceof NullLiteral and
not ma.getArgument(1).getType().getName() = ["Class<Object>", "Class<?>"]
or
isSafeFlexjsonDeserializer(ma.getQualifier())
)
)
node instanceof UnsafeDeserializationSanitizer
}
}
/** Tracks flows from remote user input to a deserialization sink. */
private module UnsafeDeserializationConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink }
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
any(UnsafeDeserializationAdditionalTaintStep s).isAdditionalTaintStep(pred, succ)
}
predicate isBarrier(DataFlow::Node node) { node instanceof UnsafeDeserializationSanitizer }
}
module UnsafeDeserializationFlow = TaintTracking::Global<UnsafeDeserializationConfig>;
/**
* Gets a safe usage of the `use` method of Flexjson, which could be:
* use(String, ...) where the path is null or
@@ -350,18 +381,9 @@ predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNo
)
}
/**
* Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance)
* passed to a deserialization method.
*
* If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type.
*/
class UnsafeTypeConfig extends TaintTracking2::Configuration {
UnsafeTypeConfig() { this = "UnsafeTypeConfig" }
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
/** A sink representing an argument of a deserialization method */
private class UnsafeTypeSink extends DataFlow::Node {
UnsafeTypeSink() {
exists(MethodAccess ma, int i, Expr arg | i > 0 and ma.getArgument(i) = arg |
(
ma.getMethod() instanceof ObjectMapperReadMethod
@@ -378,15 +400,13 @@ class UnsafeTypeConfig extends TaintTracking2::Configuration {
or
arg.getType().(RefType).hasQualifiedName("java.lang.reflect", "Type")
) and
arg = sink.asExpr()
arg = this.asExpr()
)
}
}
/**
* Holds if `fromNode` to `toNode` is a dataflow step that resolves a class
* or at least looks like resolving a class.
*/
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
private class UnsafeTypeAdditionalTaintStep extends Unit {
predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
resolveClassStep(fromNode, toNode) or
looksLikeResolveClassStep(fromNode, toNode) or
intentFlowsToParcel(fromNode, toNode)
@@ -394,9 +414,57 @@ class UnsafeTypeConfig extends TaintTracking2::Configuration {
}
/**
* DEPRECATED: Use `UnsafeTypeFlow` instead.
*
* Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance)
* passed to a deserialization method.
*
* If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type.
*/
deprecated class UnsafeTypeConfig extends TaintTracking2::Configuration {
UnsafeTypeConfig() { this = "UnsafeTypeConfig" }
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeTypeSink }
/**
* Holds if `fromNode` to `toNode` is a dataflow step that resolves a class
* or at least looks like resolving a class.
*/
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
any(UnsafeTypeAdditionalTaintStep s).isAdditionalTaintStep(fromNode, toNode)
}
}
private module UnsafeTypeConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeTypeSink }
/**
* Holds if `fromNode` to `toNode` is a dataflow step that resolves a class
* or at least looks like resolving a class.
*/
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
any(UnsafeTypeAdditionalTaintStep s).isAdditionalTaintStep(fromNode, toNode)
}
}
/**
* Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance)
* passed to a deserialization method.
*
* If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type.
*/
module UnsafeTypeFlow = TaintTracking::Global<UnsafeTypeConfig>;
/**
* DEPRECATED: Use `EnableJacksonDefaultTypingFlow` instead.
*
* Tracks flow from `enableDefaultTyping` calls to a subsequent Jackson deserialization method call.
*/
class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration {
deprecated class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration {
EnableJacksonDefaultTypingConfig() { this = "EnableJacksonDefaultTypingConfig" }
override predicate isSource(DataFlow::Node src) {
@@ -406,13 +474,43 @@ class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration {
override predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier }
}
private module EnableJacksonDefaultTypingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
any(EnableJacksonDefaultTyping ma).getQualifier() = src.asExpr()
}
predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier }
}
/**
* Tracks flow from `enableDefaultTyping` calls to a subsequent Jackson deserialization method call.
*/
module EnableJacksonDefaultTypingFlow = DataFlow::Global<EnableJacksonDefaultTypingConfig>;
/** Dataflow step that creates an `ObjectMapper` via a builder. */
private class ObjectMapperBuilderAdditionalFlowStep extends Unit {
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
m.getDeclaringType() instanceof MapperBuilder and
m.getReturnType()
.(RefType)
.hasQualifiedName("com.fasterxml.jackson.databind.json",
["JsonMapper$Builder", "JsonMapper"]) and
fromNode.asExpr() = ma.getQualifier() and
ma = toNode.asExpr()
)
}
}
/**
* DEPRECATED: Use `SafeObjectMapperFlow` instead.
*
* Tracks flow from calls that set a type validator to a subsequent Jackson deserialization method call,
* including across builder method calls.
*
* Such a Jackson deserialization method call is safe because validation will likely prevent instantiating unexpected types.
*/
class SafeObjectMapperConfig extends DataFlow2::Configuration {
deprecated class SafeObjectMapperConfig extends DataFlow2::Configuration {
SafeObjectMapperConfig() { this = "SafeObjectMapperConfig" }
override predicate isSource(DataFlow::Node src) {
@@ -426,18 +524,32 @@ class SafeObjectMapperConfig extends DataFlow2::Configuration {
* that configures or creates an `ObjectMapper` via a builder.
*/
override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
m.getDeclaringType() instanceof MapperBuilder and
m.getReturnType()
.(RefType)
.hasQualifiedName("com.fasterxml.jackson.databind.json",
["JsonMapper$Builder", "JsonMapper"]) and
fromNode.asExpr() = ma.getQualifier() and
ma = toNode.asExpr()
)
any(ObjectMapperBuilderAdditionalFlowStep s).isAdditionalFlowStep(fromNode, toNode)
}
}
private module SafeObjectMapperConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof SetPolymorphicTypeValidatorSource }
predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier }
/**
* Holds if `fromNode` to `toNode` is a dataflow step
* that configures or creates an `ObjectMapper` via a builder.
*/
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
any(ObjectMapperBuilderAdditionalFlowStep s).isAdditionalFlowStep(fromNode, toNode)
}
}
/**
* Tracks flow from calls that set a type validator to a subsequent Jackson deserialization method call,
* including across builder method calls.
*
* Such a Jackson deserialization method call is safe because validation will likely prevent instantiating unexpected types.
*/
module SafeObjectMapperFlow = DataFlow::Global<SafeObjectMapperConfig>;
/**
* A method that configures Jodd's JsonParser, either enabling dangerous deserialization to
* arbitrary Java types or restricting the types that can be instantiated.
@@ -454,20 +566,12 @@ private class JoddJsonParserConfigurationMethodQualifier extends DataFlow::ExprN
}
}
/**
* Configuration tracking flow from methods that configure `jodd.json.JsonParser`'s class
* instantiation feature to a `.parse` call on the same parser.
*/
private class JoddJsonParserConfigurationMethodConfig extends DataFlow2::Configuration {
JoddJsonParserConfigurationMethodConfig() {
this = "UnsafeDeserialization::JoddJsonParserConfigurationMethodConfig"
}
override predicate isSource(DataFlow::Node src) {
private module JoddJsonParserConfigurationMethodConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) {
src instanceof JoddJsonParserConfigurationMethodQualifier
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodAccess ma |
ma.getMethod() instanceof JoddJsonParseMethod and
sink.asExpr() = ma.getQualifier() // The class type argument
@@ -475,6 +579,13 @@ private class JoddJsonParserConfigurationMethodConfig extends DataFlow2::Configu
}
}
/**
* Configuration tracking flow from methods that configure `jodd.json.JsonParser`'s class
* instantiation feature to a `.parse` call on the same parser.
*/
private module JoddJsonParserConfigurationMethodFlow =
DataFlow::Global<JoddJsonParserConfigurationMethodConfig>;
/**
* Gets the qualifier to a method call that configures a `jodd.json.JsonParser` instance unsafely.
*
@@ -512,10 +623,8 @@ private DataFlow::Node getASafelyConfiguredParser() {
* and which never appears to be configured safely.
*/
private predicate joddJsonParserConfiguredUnsafely(Expr parserExpr) {
exists(DataFlow::Node parser, JoddJsonParserConfigurationMethodConfig config |
parser.asExpr() = parserExpr
|
config.hasFlow(getAnUnsafelyConfiguredParser(), parser) and
not config.hasFlow(getASafelyConfiguredParser(), parser)
exists(DataFlow::Node parser | parser.asExpr() = parserExpr |
JoddJsonParserConfigurationMethodFlow::flow(getAnUnsafelyConfiguredParser(), parser) and
not JoddJsonParserConfigurationMethodFlow::flow(getASafelyConfiguredParser(), parser)
)
}

6
java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql
View File

@@ -13,9 +13,9 @@
import java
import semmle.code.java.security.UnsafeDeserializationQuery
import DataFlow::PathGraph
import UnsafeDeserializationFlow::PathGraph
from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf
where conf.hasFlowPath(source, sink)
from UnsafeDeserializationFlow::PathNode source, UnsafeDeserializationFlow::PathNode sink
where UnsafeDeserializationFlow::flowPath(source, sink)
select sink.getNode().(UnsafeDeserializationSink).getMethodAccess(), source, sink,
"Unsafe deserialization depends on a $@.", source.getNode(), "user-provided value"

2
java/ql/test/query-tests/security/CWE-502/UnsafeDeserialization.ql
View File

@@ -9,7 +9,7 @@ class UnsafeDeserializationTest extends InlineExpectationsTest {
override predicate hasActualResult(Location location, string element, string tag, string value) {
tag = "unsafeDeserialization" and
exists(DataFlow::Node sink, UnsafeDeserializationConfig conf | conf.hasFlowTo(sink) |
exists(DataFlow::Node sink | UnsafeDeserializationFlow::flowTo(sink) |
sink.getLocation() = location and
element = sink.toString() and
value = ""