Add Gson support to unsafe deserialization query

Author: smowton

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

@@ -77,6 +77,8 @@ private import FlowSummary
  */
 private module Frameworks {
   private import internal.ContainerFlow
+  private import semmle.code.java.frameworks.android.Android
+  private import semmle.code.java.frameworks.android.Intent
   private import semmle.code.java.frameworks.android.XssSinks
   private import semmle.code.java.frameworks.android.Intent
   private import semmle.code.java.frameworks.ApacheHttp

java/ql/lib/semmle/code/java/frameworks/android/Android.qll

@@ -5,6 +5,7 @@
 import java
 import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.xml.AndroidManifest
+private import semmle.code.java.dataflow.ExternalFlow
 
 /**
  * Gets a transitive superType avoiding magic optimisation
@@ -202,3 +203,59 @@ private class ContentProviderSourceModels extends SourceModelCsv {
       ]
   }
 }
+
+/** Interface for classes whose instances can be written to and restored from a Parcel. */
+class TypeParcelable extends Interface {
+  TypeParcelable() { this.hasQualifiedName("android.os", "Parcelable") }
+}
+
+/**
+ * A method that overrides `android.os.Parcelable.Creator.createFromParcel`.
+ */
+class CreateFromParcelMethod extends Method {
+  CreateFromParcelMethod() {
+    this.hasName("createFromParcel") and
+    this.getEnclosingCallable().getDeclaringType().getASupertype*() instanceof TypeParcelable
+  }
+}
+
+private class TaintPropagationModels extends SummaryModelCsv {
+  override predicate row(string s) {
+    // BaseBundle getters
+    s =
+      "android.os;BaseBundle;true;get" + ["Boolean", "Double", "Int", "Long", "String"] +
+        ["", "Array"] + ";;;Argument[-1];ReturnValue;taint"
+    or
+    // Bundle getters
+    s =
+      "android.os;Bundle;true;get" +
+        [
+          "Binder", "Bundle", "Byte", "ByteArray", "Char", "CharArray", "CharSequence",
+          "CharSequenceArray", "CharSequenceArrayList", "Float", "FloatArray", "IntegerArrayList",
+          "Parcelable", "ParcelableArray", "ParcelableArrayList", "Serializable", "Short",
+          "ShortArray", "Size", "SizeF", "SparseParcelableArray", "StringArrayList"
+        ] + ";;;Argument[-1];ReturnValue;taint"
+    or
+    // Intent readers that return their value
+    s =
+      "android.os;Parcel;false;read" +
+        [
+          "Array", "ArrayList", "Boolean", "Bundle", "Byte", "Double", "FileDescriptor", "Float",
+          "HashMap", "Int", "Long", "Parcelable", "ParcelableArray", "PersistableBundle",
+          "Serializable", "Size", "SizeF", "SparseArray", "SparseBolleanArray", "String",
+          "StrongBinder", "TypedObject", "Value"
+        ] + ";;;Argument[-1];ReturnValue;taint"
+    or
+    // Intent readers that write to an existing object
+    s =
+      "android.os;Parcel;false;read" +
+        [
+          "BinderArray", "BinderList", "BooleanArray", "ByteArray", "CharArray", "DoubleArray",
+          "FloatArray", "IntArray", "List", "LongArray", "Map", "ParcelableList", "StringArray",
+          "StringList", "TypedArray", "TypedList"
+        ] + ";;;Argument[-1];Argument[0];taint"
+    or
+    // One Intent method that aliases an argument to a return value
+    s = "android.os;Parcel;false;readParcelableList;;;Argument[0];ReturnValue;value"
+  }
+}

java/ql/lib/semmle/code/java/frameworks/android/Intent.qll

@@ -3,32 +3,53 @@ private import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.FlowSteps
 import semmle.code.java.dataflow.ExternalFlow
 
+/**
+ * The class `android.content.Intent`.
+ */
 class TypeIntent extends Class {
   TypeIntent() { hasQualifiedName("android.content", "Intent") }
 }
 
+/**
+ * The class `android.app.Activity`.
+ */
 class TypeActivity extends Class {
   TypeActivity() { hasQualifiedName("android.app", "Activity") }
 }
 
+/**
+ * The class `android.content.Context`.
+ */
 class TypeContext extends RefType {
   TypeContext() { hasQualifiedName("android.content", "Context") }
 }
 
+/**
+ * The class `android.content.BroadcastReceiver`.
+ */
 class TypeBroadcastReceiver extends Class {
   TypeBroadcastReceiver() { hasQualifiedName("android.content", "BroadcastReceiver") }
 }
 
+/**
+ * The method `Activity.getIntent`
+ */
 class AndroidGetIntentMethod extends Method {
   AndroidGetIntentMethod() { hasName("getIntent") and getDeclaringType() instanceof TypeActivity }
 }
 
+/**
+ * The method `BroadcastReceiver.onReceive`.
+ */
 class AndroidReceiveIntentMethod extends Method {
   AndroidReceiveIntentMethod() {
     hasName("onReceive") and getDeclaringType() instanceof TypeBroadcastReceiver
   }
 }
 
+/**
+ * The method `Context.startActivity` or `startActivities`.
+ */
 class ContextStartActivityMethod extends Method {
   ContextStartActivityMethod() {
     (hasName("startActivity") or hasName("startActivities")) and
@@ -44,6 +65,16 @@ private class IntentFieldsInheritTaint extends DataFlow::SyntheticFieldContent,
   IntentFieldsInheritTaint() { this.getField().matches("android.content.Intent.%") }
 }
 
+/**
+ * The method `Intent.getParcelableExtra`.
+ */
+class IntentGetParcelableExtraMethod extends Method {
+  IntentGetParcelableExtraMethod() {
+    hasName("getParcelableExtra") and
+    getDeclaringType() instanceof TypeIntent
+  }
+}
+
 private class IntentBundleFlowSteps extends SummaryModelCsv {
   override predicate row(string row) {
     row =

java/ql/lib/semmle/code/java/frameworks/google/Gson.qll

@@ -0,0 +1,35 @@
+/**
+ * Provides classes for working with the Gson framework.
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.frameworks.android.Android
+import semmle.code.java.frameworks.android.Intent
+
+/** The class `com.google.gson.Gson`. */
+class Gson extends RefType {
+  Gson() { this.hasQualifiedName("com.google.gson", "Gson") }
+}
+
+/** The `fromJson` deserialization method. */
+class GsonDeserializeMethod extends Method {
+  GsonDeserializeMethod() {
+    this.getDeclaringType() instanceof Gson and
+    this.hasName("fromJson")
+  }
+}
+
+/**
+ * Holds if `intentNode` is an `Intent` used in the context `(T)intentNode.getParcelableExtra(...)` and
+ * `parcelNode` is the corresponding parameter of `Parcelable.Creator<T> { public T createFromParcel(Parcel parcelNode) { }`.
+ */
+predicate intentFlowsToParcel(DataFlow::Node intentNode, DataFlow::Node parcelNode) {
+  exists(MethodAccess getParcelableExtraCall, CreateFromParcelMethod cfpm, Type createdType |
+    intentNode.asExpr() = getParcelableExtraCall.getQualifier() and
+    getParcelableExtraCall.getMethod() instanceof IntentGetParcelableExtraMethod and
+    DataFlow::localExprFlow(getParcelableExtraCall, any(Expr e | e.getType() = createdType)) and
+    parcelNode.asParameter() = cfpm.getParameter(0) and
+    cfpm.getReturnType() = createdType
+  )
+}

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

@@ -17,6 +17,7 @@ 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.google.Gson
 private import semmle.code.java.frameworks.apache.Lang
 private import semmle.code.java.Reflection
 
@@ -207,6 +208,10 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
     or
     m instanceof FlexjsonDeserializeMethod and
     sink = ma.getArgument(0)
+    or
+    m instanceof GsonDeserializeMethod and
+    sink = ma.getArgument(0) and
+    any(UnsafeTypeConfig config).hasFlowToExpr(ma.getArgument(1))
   )
 }
 
@@ -249,6 +254,8 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
     createJacksonJsonParserStep(pred, succ)
     or
     createJacksonTreeNodeStep(pred, succ)
+    or
+    intentFlowsToParcel(pred, succ)
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -362,9 +369,15 @@ class UnsafeTypeConfig extends TaintTracking2::Configuration {
         ma.getMethod() instanceof JabsorbUnmarshallMethod
         or
         ma.getMethod() instanceof JoddJsonParseMethod
+        or
+        ma.getMethod() instanceof GsonDeserializeMethod
       ) and
       // Note `JacksonTypeDescriptorType` includes plain old `java.lang.Class`
-      arg.getType() instanceof JacksonTypeDescriptorType and
+      (
+        arg.getType() instanceof JacksonTypeDescriptorType
+        or
+        arg.getType().(RefType).hasQualifiedName("java.lang.reflect", "Type")
+      ) and
       arg = sink.asExpr()
     )
   }
@@ -375,7 +388,8 @@ class UnsafeTypeConfig extends TaintTracking2::Configuration {
    */
   override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
     resolveClassStep(fromNode, toNode) or
-    looksLikeResolveClassStep(fromNode, toNode)
+    looksLikeResolveClassStep(fromNode, toNode) or
+    intentFlowsToParcel(fromNode, toNode)
   }
 }
 

java/ql/test/query-tests/security/CWE-502/AndroidManifest.xml

@@ -0,0 +1,24 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.example.app"
+    android:installLocation="auto"
+    android:versionCode="1"
+    android:versionName="0.1" >
+
+    <uses-permission android:name="android.permission.INTERNET" />
+
+    <application
+        android:icon="@drawable/ic_launcher"
+        android:label="@string/app_name"
+        android:theme="@style/AppTheme" >
+        <activity
+            android:name=".GsonActivity"
+            android:icon="@drawable/ic_launcher"
+			android:label="@string/app_name">
+            <intent-filter>
+                <action android:name="android.intent.action.MAIN" />
+                <category android:name="android.intent.category.LAUNCHER" />
+            </intent-filter>
+        </activity>
+    </application>
+
+</manifest>

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

@@ -0,0 +1,17 @@
+package com.example.app;
+
+import android.app.Activity;
+import android.os.Bundle;
+import android.os.Parcel;
+import android.os.Parcelable;
+
+import com.google.gson.Gson; 
+
+public class GsonActivity extends Activity {
+    public void onCreate(Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        setContentView(-1);
+
+        ParcelableEntity entity = (ParcelableEntity) getIntent().getParcelableExtra("jsonEntity");
+    }
+}

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

@@ -0,0 +1,77 @@
+import java.io.IOException;
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.typeadapters.RuntimeTypeAdapterFactory;
+
+import com.example.User;
+import com.thirdparty.Person;
+
+public class GsonServlet extends HttpServlet {
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    // GOOD: concrete class type specified
+    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+
+        Gson gson = new Gson();
+        Object obj = gson.fromJson(json, User.class);
+    }
+
+    @Override
+    // GOOD: concrete class type specified
+    public void doPut(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+
+        Gson gson = new Gson();
+        Object obj = gson.fromJson(json, Person.class);
+    }
+
+    @Override
+    // BAD: allow class name to be controlled by remote source
+    public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        String clazz = req.getParameter("class");
+
+        try {
+            Gson gson = new Gson();
+            Object obj = gson.fromJson(json, Class.forName(clazz)); // $unsafeDeserialization
+        } catch (ClassNotFoundException cne) {
+            throw new IOException(cne.getMessage());
+        }
+    }
+
+    @Override
+    // BAD: allow class name to be controlled by remote source even with a type adapter factory
+    public void doHead(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        String clazz = req.getParameter("class");
+
+        try {
+            RuntimeTypeAdapterFactory<User> runtimeTypeAdapterFactory = RuntimeTypeAdapterFactory
+            .of(User.class, "type");
+            Gson gson = new GsonBuilder().registerTypeAdapterFactory(runtimeTypeAdapterFactory).create();
+            Object obj = gson.fromJson(json, Class.forName(clazz)); // $unsafeDeserialization
+        } catch (ClassNotFoundException cne) {
+            throw new IOException(cne.getMessage());
+        }
+    }
+
+    @Override
+    // GOOD: specify allowed class types without explicitly configured vulnerable subclass types
+    public void doTrace(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+        String json = req.getParameter("json");
+        String clazz = req.getParameter("class");
+
+        RuntimeTypeAdapterFactory<Person> runtimeTypeAdapterFactory = RuntimeTypeAdapterFactory
+            .of(Person.class, "type");
+        Gson gson = new GsonBuilder().registerTypeAdapterFactory(runtimeTypeAdapterFactory).create();
+        Person obj = gson.fromJson(json, Person.class);
+    }
+}