Add support for the Flexjson framework to the unsafe-deserialization query

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

@@ -81,6 +81,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.ApacheHttp
   private import semmle.code.java.frameworks.apache.Collections
   private import semmle.code.java.frameworks.apache.Lang
+  private import semmle.code.java.frameworks.Flexjson
   private import semmle.code.java.frameworks.guava.Guava
   private import semmle.code.java.frameworks.jackson.JacksonSerializability
   private import semmle.code.java.frameworks.JavaxJson

java/ql/lib/semmle/code/java/frameworks/Flexjson.qll

@@ -0,0 +1,41 @@
+/**
+ * Provides classes for working with the Flexjson framework.
+ */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+
+/** The class `flexjson.JSONDeserializer`. */
+class FlexjsonDeserializer extends RefType {
+  FlexjsonDeserializer() { this.hasQualifiedName("flexjson", "JSONDeserializer") }
+}
+
+/** The class `flexjson.ObjectFactory`. */
+class FlexjsonObjectFactory extends RefType {
+  FlexjsonObjectFactory() { this.hasQualifiedName("flexjson", "ObjectFactory") }
+}
+
+/** The deserialization method `deserialize`. */
+class FlexjsonDeserializeMethod extends Method {
+  FlexjsonDeserializeMethod() {
+    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof
+      FlexjsonDeserializer and
+    this.getName() = "deserialize" and
+    not this.getAParameter().getType() instanceof FlexjsonObjectFactory // deserialization method with specified class types in object factory is unlikely to be vulnerable
+  }
+}
+
+/** The method `use` to configure allowed class type. */
+class FlexjsonDeserializerUseMethod extends Method {
+  FlexjsonDeserializerUseMethod() {
+    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof
+      FlexjsonDeserializer and
+    this.hasName("use")
+  }
+}
+
+private class FluentUseMethodModel extends SummaryModelCsv {
+  override predicate row(string r) {
+    r = "flexjson;JSONDeserializer;false;use;;;Argument[-1];ReturnValue;value"
+  }
+}

java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll

@@ -16,6 +16,7 @@ private import semmle.code.java.frameworks.Castor
 private import semmle.code.java.frameworks.Jackson
 private import semmle.code.java.frameworks.Jabsorb
 private import semmle.code.java.frameworks.JoddJson
+private import semmle.code.java.frameworks.Flexjson
 private import semmle.code.java.frameworks.apache.Lang
 private import semmle.code.java.Reflection
 
@@ -203,6 +204,9 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
       // jodd.json.JsonParser may be configured for unrestricted deserialization to user-specified types
       joddJsonParserConfiguredUnsafely(ma.getQualifier())
     )
+    or
+    m instanceof FlexjsonDeserializeMethod and
+    sink = ma.getArgument(0)
   )
 }
 
@@ -270,9 +274,48 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
       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())
+      )
+    )
   }
 }
 
+/**
+ * Gets a safe usage of the `use` method of Flexjson, which could be:
+ *     use(String, ...) where the path is null or
+ *     use(ObjectFactory, String...) where the string varargs (or array) contains null
+ */
+MethodAccess getASafeFlexjsonUseCall() {
+  result.getMethod() instanceof FlexjsonDeserializerUseMethod and
+  (
+    result.getMethod().getParameterType(0) instanceof TypeString and
+    result.getArgument(0) instanceof NullLiteral
+    or
+    result.getMethod().getParameterType(0) instanceof FlexjsonObjectFactory and
+    exists(NullLiteral e | e = result.getAnArgument())
+  )
+}
+
+/**
+ * Holds if `e` is a safely configured Flexjson `JSONDeserializer`.
+ */
+predicate isSafeFlexjsonDeserializer(Expr e) {
+  DataFlow::localExprFlow(getASafeFlexjsonUseCall().getQualifier(), e)
+}
+
 /** Holds if `fromNode` to `toNode` is a dataflow step that resolves a class. */
 predicate resolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
   exists(ReflectiveClassIdentifierMethodAccess ma |

java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.qhelp

@@ -15,7 +15,7 @@ may have unforeseen effects, such as the execution of arbitrary code.
 <p>
 There are many different serialization frameworks.  This query currently
 supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap,
-Jackson, Jabsorb, Jodd JSON and Java IO serialization through
+Jackson, Jabsorb, Jodd JSON, Flexjson and Java IO serialization through
 <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
 </p>
 </overview>
@@ -109,6 +109,10 @@ Jabsorb documentation on deserialization:
 Jodd JSON documentation on deserialization:
 <a href="https://json.jodd.org/parser">JoddJson Parser</a>.
 </li>
+<li>
+RCE in Flexjson:
+<a href="https://codewhitesec.blogspot.com/2020/03/liferay-portal-json-vulns.html">Flexjson deserialization</a>.
+</li>
 </references>
 
 </qhelp>

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

@@ -0,0 +1,126 @@
+import java.io.IOException;
+import java.io.Reader;
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import flexjson.JSONDeserializer;
+import flexjson.factories.ExistingObjectFactory;
+
+import com.example.User;
+import com.thirdparty.Person;
+
+public class FlexjsonServlet extends HttpServlet {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    // GOOD: a final class type is specified
+    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        JSONDeserializer<User> deserializer = new JSONDeserializer<>();
+        User user = deserializer.deserialize(req.getReader(), User.class);
+    }
+
+    @Override
+    // GOOD: a non-null class type is specified which is not the generic `Object` type
+    public void doHead(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        JSONDeserializer<Person> deserializer = new JSONDeserializer<Person>();
+        Person person = deserializer.deserialize(req.getReader(), Person.class);
+    }
+
+    @Override
+    // BAD: allow class name to be controlled by remote source
+    public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        JSONDeserializer<User> deserializer = new JSONDeserializer<>();
+        User user = (User) deserializer.deserialize(req.getReader()); // $unsafeDeserialization
+
+    }
+
+    @Override
+    // BAD: allow class name to be controlled by remote source
+    public void doTrace(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        JSONDeserializer deserializer = new JSONDeserializer<>();
+        User user = (User) deserializer.deserialize(req.getReader()); // $unsafeDeserialization
+
+    }
+
+    @Override
+    // BAD: specify overly generic class type
+    public void doPut(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        JSONDeserializer deserializer = new JSONDeserializer();
+        User user = (User) deserializer.deserialize(req.getReader(), Object.class); // $unsafeDeserialization
+    }
+
+    private Person fromJsonToPerson(String json) {
+        return new JSONDeserializer<Person>().use(null, Person.class).deserialize(json);
+    }
+
+    // GOOD: Specify the class to deserialize with `use`
+    public void doPut2(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = fromJsonToPerson(json);
+    }
+
+    // BAD: Specify a concrete class type to `use` with `ObjectFactory`
+    public void doPut3(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().use(Person.class, new ExistingObjectFactory(new Person())).deserialize(json); // $unsafeDeserialization
+    }
+
+    // GOOD: Specify a null path to `use` with a concrete class type
+    public void doPut4(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().use(null, Person.class).deserialize(json);
+    }
+
+    // BAD: Specify a non-null json path to `use` with a concrete class type
+    public void doPut5(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().use("abc", Person.class).deserialize(json); // $unsafeDeserialization
+    }
+
+    // GOOD: Specify a null json path to `use` with `ObjectFactory`
+    public void doPut6(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().use(new ExistingObjectFactory(new Person()), "abc", null).deserialize(json);
+    }
+
+    // GOOD: Specify a concrete class type to deserialize with `ObjectFactory`
+    public void doPut7(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().deserialize(json, new ExistingObjectFactory(new Person()));
+    }
+
+    // GOOD: Specify the class type to deserialize into
+    public void doPut8(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().deserializeInto(json, new Person());
+    }
+
+    // GOOD: Specify a null json path to `use` with a concrete class type, interwoven with irrelevant use directives
+    public void doPut9(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        Person person = new JSONDeserializer<Person>().use(Person.class, null).use(null, Person.class).use(String.class, null).deserialize(json);
+    }
+
+    // GOOD: Specify a null json path to `use` with a concrete class type, interwoven with irrelevant use directives, without using fluent method chaining
+    public void doPut10(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        JSONDeserializer<Person> deserializer = new JSONDeserializer<Person>();
+        deserializer.use(Person.class, null);
+        deserializer.use(null, Person.class);
+        deserializer.use(String.class, null);
+        Person person = deserializer.deserialize(json);
+    }
+
+    // BAD: Specify a non-null json path to `use` with a concrete class type, interwoven with irrelevant use directives, without using fluent method chaining
+    public void doPut11(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        JSONDeserializer<Person> deserializer = new JSONDeserializer<Person>();
+        deserializer.use(Person.class, null);
+        deserializer.use("someKey", Person.class);
+        deserializer.use(String.class, null);
+        Person person = deserializer.deserialize(json); // $unsafeDeserialization
+    }
+}

java/ql/test/query-tests/security/CWE-502/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/snakeyaml-1.21:${testdir}/../../../stubs/xstream-1.4.10:${testdir}/../../../stubs/kryo-4.0.2:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/fastjson-1.2.74:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/jyaml-1.3:${testdir}/../../../stubs/json-io-4.10.0:${testdir}/../../../stubs/yamlbeans-1.09:${testdir}/../../../stubs/hessian-4.0.38:${testdir}/../../../stubs/castor-1.4.1:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/jabsorb-1.3.2:${testdir}/../../../stubs/json-java-20210307:${testdir}/../../../stubs/joddjson-6.0.3
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/snakeyaml-1.21:${testdir}/../../../stubs/xstream-1.4.10:${testdir}/../../../stubs/kryo-4.0.2:${testdir}/../../../stubs/jsr311-api-1.1.1:${testdir}/../../../stubs/fastjson-1.2.74:${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/jyaml-1.3:${testdir}/../../../stubs/json-io-4.10.0:${testdir}/../../../stubs/yamlbeans-1.09:${testdir}/../../../stubs/hessian-4.0.38:${testdir}/../../../stubs/castor-1.4.1:${testdir}/../../../stubs/jackson-databind-2.12:${testdir}/../../../stubs/jackson-core-2.12:${testdir}/../../../stubs/jabsorb-1.3.2:${testdir}/../../../stubs/json-java-20210307:${testdir}/../../../stubs/joddjson-6.0.3:${testdir}/../../../stubs/flexjson-2.1

java/ql/test/stubs/flexjson-2.1/flexjson/JSONDeserializer.java

@@ -0,0 +1,121 @@
+package flexjson;
+
+import java.io.Reader;
+
+public class JSONDeserializer<T> {
+
+    public JSONDeserializer() {
+    }
+
+    /**
+     * Deserialize the given json formatted input into a Java object.
+     *
+     * @param input a json formatted string.
+     * @return an Java instance deserialized from the json input.
+     */
+    public T deserialize( String input ) {
+        return null;
+    }
+
+    /**
+     * Same as {@link #deserialize(String)}, but uses an instance of
+     * java.io.Reader as json input.
+     *
+     * @param input the stream where the json input is coming from.
+     * @return an Java instance deserialized from the java.io.Reader's input.
+     */
+    public T deserialize( Reader input ) {
+        return null;
+    }
+
+    /**
+     * Deserialize the given json input, and use the given Class as
+     * the type of the initial object to deserialize into.  This object
+     * must implement a no-arg constructor.
+     *
+     * @param input a json formatted string.
+     * @param root a Class used to create the initial object.
+     * @return the object created from the given json input.
+     */
+    public T deserialize( String input, Class root ) {
+        return null;
+    }
+
+    /**
+     * Same as {@link #deserialize(java.io.Reader, Class)}, but uses an instance of
+     * java.io.Reader as json input.
+     *
+     * @param input the stream where the json input is coming from.
+     * @param root a Class used to create the initial object.
+     * @return an Java instance deserialized from the java.io.Reader's input.
+     */
+    public T deserialize( Reader input, Class root ) {
+        return null;
+    }
+
+    /**
+     * Deserialize the given json input, and use the given ObjectFactory to
+     * create the initial object to deserialize into.
+     *
+     * @param input a json formatted string.
+     * @param factory an ObjectFactory used to create the initial object.
+     * @return the object created from the given json input.
+     */
+    public T deserialize( String input, ObjectFactory factory ) {
+        return null;
+    }
+
+    /**
+     * Same as {@link #deserialize(String, ObjectFactory)}, but uses an instance of
+     * java.io.Reader as json input.
+     *
+     * @param input the stream where the json input is coming from.
+     * @param factory an ObjectFactory used to create the initial object.
+     * @return an Java instance deserialized from the java.io.Reader's input.
+     */
+    public T deserialize( Reader input, ObjectFactory factory ) {
+        return null;
+    }
+
+    /**
+     * Deserialize the given input into the existing object target.
+     * Values in the json input will overwrite values in the
+     * target object.  This means if a value is included in json
+     * a new object will be created and set into the existing object. 
+     *
+     * @param input a json formatted string.
+     * @param target an instance to set values into from the json string.
+     * @return will return a reference to target.
+     */
+    public T deserializeInto( String input, T target ) {
+        return null;
+    }
+
+    /**
+     * Same as {@link #deserializeInto(String, Object)}, but uses an instance of
+     * java.io.Reader as json input.
+     *
+     * @param input the stream where the json input is coming from.
+     * @param target an instance to set values into from the json string.
+     * @return will return a reference to target.
+     */
+    public T deserializeInto( Reader input, T target ) {
+        return null;
+    }
+
+    public JSONDeserializer<T> use( String path, Class clazz ) {
+        return null;
+    }
+
+    public JSONDeserializer<T> use( Class clazz, ObjectFactory factory ) {
+        return null;
+    }
+
+    public JSONDeserializer<T> use( String path, ObjectFactory factory ) {
+        return null;
+    }
+
+    public JSONDeserializer<T> use(ObjectFactory factory, String... paths) {
+        return null;
+    }
+}

java/ql/test/stubs/flexjson-2.1/flexjson/ObjectBinder.java

@@ -0,0 +1,40 @@
+package flexjson;
+
+import java.lang.reflect.Type;
+
+import java.util.Map;
+import java.util.Collection;
+
+public class ObjectBinder {
+
+    public ObjectBinder() {
+    }
+
+    public ObjectBinder use(Class clazz, ObjectFactory factory) {
+        return null;
+    }
+
+    public Object bind( Object input ) {
+        return null;
+    }
+
+    public Object bind( Object source, Object target ) {
+        return null;
+    }
+
+    public Object bind( Object input, Type targetType ) {
+        return null;
+    }
+
+    public <T extends Collection<Object>> T bindIntoCollection(Collection value, T target, Type targetType) {
+        return null;
+    }
+
+    public Object bindIntoMap(Map input, Map<Object, Object> result, Type keyType, Type valueType) {
+        return null;
+    }
+
+    public Object bindIntoObject(Map jsonOwner, Object target, Type targetType) {
+        return null;
+    }
+}

java/ql/test/stubs/flexjson-2.1/flexjson/ObjectFactory.java

@@ -0,0 +1,26 @@
+package flexjson;
+
+import java.lang.reflect.Type;
+
+/**
+ * ObjectFactory allows you to instantiate specific types of objects on path or class types.  This interface allows
+ * you to override the default rules.  
+ */
+public interface ObjectFactory {
+    /**
+     * This method is called by the deserializer to construct and bind an object.  At the end of this method
+     * the object should be fully constructed.  {@link flexjson.ObjectBinder} can be used to bind values into
+     * the object according to default rules.  For simple implementations you won't need to use this, but
+     * for more complex or generic objects reusing methods like {@link flexjson.ObjectBinder#bind(Object, java.lang.reflect.Type)}
+     * and {@link flexjson.ObjectBinder#bindIntoCollection(java.util.Collection, java.util.Collection, java.lang.reflect.Type)}.
+     *
+     * @param context the object binding context to keep track of where we are in the object graph
+     * and used for binding into objects.
+     * @param value This is the value from the json object at the current path.
+     * @param targetType This is the type pulled from the object introspector.  Used for Collections and generic types.
+     * @param targetClass concrete class pulled from the configuration of the deserializer.
+     *
+     * @return the fully bound object.  At the end of this method the object should be fully constructed.
+     */
+    public Object instantiate(ObjectBinder context, Object value, Type targetType, Class targetClass);
+}

java/ql/test/stubs/flexjson-2.1/flexjson/factories/ExistingObjectFactory.java

@@ -0,0 +1,17 @@
+package flexjson.factories;
+
+import flexjson.ObjectBinder;
+import flexjson.ObjectFactory;
+
+import java.lang.reflect.Type;
+
+public class ExistingObjectFactory implements ObjectFactory {
+
+    public ExistingObjectFactory(Object source) {
+    }
+
+    @Override
+    public Object instantiate(ObjectBinder context, Object value, Type targetType, Class targetClass) {
+        return null;
+    }
+}