Skip to content

Commit 97fca62

Browse files
Cover attacker-controlled types for deserialization with Jackson
1 parent 3eb2af1 commit 97fca62

1 file changed

Lines changed: 46 additions & 4 deletions

File tree

java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import semmle.code.java.dataflow.FlowSources
2+
import semmle.code.java.dataflow.TaintTracking2
13
import semmle.code.java.frameworks.Kryo
24
import semmle.code.java.frameworks.XStream
35
import semmle.code.java.frameworks.SnakeYaml
@@ -31,7 +33,9 @@ class ObjectMapperReadMethod extends Method {
3133
}
3234

3335
class ObjectMapper extends RefType {
34-
ObjectMapper() { hasQualifiedName("com.fasterxml.jackson.databind", "ObjectMapper") }
36+
ObjectMapper() {
37+
getASupertype*().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectMapper")
38+
}
3539
}
3640

3741
class MapperBuilder extends RefType {
@@ -48,6 +52,14 @@ class JsonParser extends RefType {
4852
JsonParser() { hasQualifiedName("com.fasterxml.jackson.core", "JsonParser") }
4953
}
5054

55+
class JacksonType extends RefType {
56+
JacksonType() {
57+
this instanceof TypeClass or
58+
hasQualifiedName("com.fasterxml.jackson.databind", "JavaType") or
59+
hasQualifiedName("com.fasterxml.jackson.core.type", "TypeReference")
60+
}
61+
}
62+
5163
class EnableJacksonDefaultTyping extends MethodAccess {
5264
EnableJacksonDefaultTyping() {
5365
this.getMethod().getDeclaringType() instanceof ObjectMapper and
@@ -206,6 +218,31 @@ class SafeObjectMapper extends DataFlow2::Configuration {
206218
}
207219
}
208220

221+
class UnsafeType extends TaintTracking2::Configuration {
222+
UnsafeType() { this = "UnsafeType" }
223+
224+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
225+
226+
override predicate isSink(DataFlow::Node sink) {
227+
exists(MethodAccess ma, int i, Expr arg | i > 0 and ma.getArgument(i) = arg |
228+
ma.getMethod() instanceof ObjectMapperReadMethod and
229+
arg.getType() instanceof JacksonType and
230+
arg = sink.asExpr()
231+
)
232+
}
233+
234+
/**
235+
* Holds if `fromNode` to `toNode` is a dataflow step that looks like resolving a class.
236+
*/
237+
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
238+
exists(MethodAccess ma, RefType returnType | returnType = ma.getMethod().getReturnType() |
239+
returnType instanceof JacksonType and
240+
ma.getAnArgument() = fromNode.asExpr() and
241+
ma = toNode.asExpr()
242+
)
243+
}
244+
}
245+
209246
/**
210247
* Holds if `fromNode` to `toNode` is a dataflow step that creates a Jackson parser.
211248
*/
@@ -246,7 +283,7 @@ predicate hasFieldWithJsonTypeAnnotation(RefType type) {
246283
exists(Annotation a |
247284
type.getAField().getAnAnnotation() = a and
248285
a.getType().hasQualifiedName("com.fasterxml.jackson.annotation", "JsonTypeInfo") and
249-
a.getValue("use").(VarAccess).getVariable().hasName("CLASS")
286+
a.getValue("use").(VarAccess).getVariable().hasName(["CLASS", "MINIMAL_CLASS"])
250287
)
251288
}
252289

@@ -302,8 +339,13 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
302339
ma.getMethod() instanceof ObjectMapperReadMethod and
303340
sink = ma.getArgument(0) and
304341
(
305-
exists(EnabledJacksonDefaultTyping config | config.hasFlowToExpr(ma.getQualifier())) or
306-
hasJsonTypeInfoAnnotation(ma.getArgument(1).getType().(ParameterizedType).getATypeArgument())
342+
exists(UnsafeType config | config.hasFlowToExpr(ma.getAnArgument()))
343+
or
344+
exists(EnabledJacksonDefaultTyping config | config.hasFlowToExpr(ma.getQualifier()))
345+
or
346+
exists(RefType argType, int i | i > 0 and argType = ma.getArgument(i).getType() |
347+
hasJsonTypeInfoAnnotation(argType.(ParameterizedType).getATypeArgument())
348+
)
307349
) and
308350
not exists(SafeObjectMapper config | config.hasFlowToExpr(ma.getQualifier()))
309351
)

0 commit comments

Comments
 (0)