Better looksLikeResolveClassStep() predicate

Author: artem-smotrakov

java/ql/src/semmle/code/java/frameworks/Jackson.qll

@@ -156,19 +156,17 @@ predicate hasArgumentWithUnsafeJacksonAnnotation(MethodAccess call) {
 
 /**
  * Holds if `fromNode` to `toNode` is a dataflow step that looks like resolving a class.
- * A method probably resolves a class if it is external, takes a string, returns a type descriptor
+ * A method probably resolves a class if takes a string, returns a type descriptor,
  * and its name contains "resolve", "load", etc.
  *
  * Any method call that satisfies the rule above is assumed to propagate taint from its string arguments,
  * so methods that accept user-controlled data but sanitize it or use it for some
  * completely different purpose before returning a type descriptor could result in false positives.
  */
 predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-  exists(MethodAccess ma, Method m, int i, Expr arg |
-    m = ma.getMethod() and arg = ma.getArgument(i)
-  |
+  exists(MethodAccess ma, Method m, Expr arg | m = ma.getMethod() and arg = ma.getAnArgument() |
     m.getReturnType() instanceof JacksonTypeDescriptorType and
-    m.getName().toLowerCase().regexpMatch("resolve|load|class|type") and
+    m.getName().toLowerCase().regexpMatch("(.*)(resolve|load|class|type)(.*)") and
     arg.getType() instanceof TypeString and
     arg = fromNode.asExpr() and
     ma = toNode.asExpr()

java/ql/test/query-tests/security/CWE-502/JacksonTest.java

@@ -1,5 +1,6 @@
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.json.JsonMapper;
 import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator;
@@ -179,12 +180,12 @@ private static void testUnsafeDeserializationWithUnsafeClassAndCustomTypeResolve
             String data = parts[0];
             String type = parts[1];
             ObjectMapper mapper = new ObjectMapper();
-            mapper.readValue(data, resolveTypeImpl(type)); // $unsafeDeserialization
+            mapper.readValue(data, resolveImpl(type, mapper)); // $unsafeDeserialization
         });
     }
 
-    private static Class resolveTypeImpl(String type) throws Exception {
-        return Class.forName(type);
+    private static JavaType resolveImpl(String type, ObjectMapper mapper) throws Exception {
+        return mapper.constructType(Class.forName(type));
     }
 }
 

java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/JavaType.java

@@ -0,0 +1,3 @@
+package com.fasterxml.jackson.databind;
+
+public class JavaType {}

java/ql/test/stubs/jackson-databind-2.10/com/fasterxml/jackson/databind/ObjectMapper.java

@@ -3,6 +3,7 @@
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.TreeNode;
 import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator;
+import java.lang.reflect.Type;
 import java.io.*;
 import java.util.*;
 
@@ -54,6 +55,10 @@ public <T> T readValue(String content, Class<T> valueType) {
     return null;
   }
 
+  public <T> T readValue(String content, JavaType valueType) {
+    return null;
+  }
+
   public <T> MappingIterator<T> readValues(JsonParser p, Class<T> valueType) {
     return null;
   }
@@ -65,4 +70,8 @@ public <T> T treeToValue(TreeNode n, Class<T> valueType) {
   public JsonNode readTree(String content) {
     return null;
   }
+
+  public JavaType constructType(Type t) {
+    return null;
+  }
 }