Java: CWE-502 Unsafe JSON deserialization with Gson, Flexjson, Jabsorb and JoddJson

java/change-notes/2021-05-25-json-deserialization-sinks.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The "Deserialization of user-controlled data" (`java/unsafe-deserialization`) query
+  now recognizes deserialization of `Gson`, `Flexjson`, `JoddJson`, and `Jabsorb`.

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

@@ -13,9 +13,10 @@ craft a nested combination of objects on which the executed initialization code
 may have unforeseen effects, such as the execution of arbitrary code.
 </p>
 <p>
-There are many different serialization frameworks.  This query currently
-supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap 
-and Java IO serialization through <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
+There are many different serialization frameworks.  This query currently supports
+Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap,
+Gson, Flexjson, JoddJson, Jabsorb, and Java IO serialization through
+<code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
 </p>
 </overview>
 
@@ -91,6 +92,22 @@ Remote code execution in JYaml library:
 JsonIO deserialization vulnerabilities:
 <a href="https://klezvirus.github.io/Advanced-Web-Hacking/Serialisation/">JsonIO deserialization</a>.
 </li>
+<li>
+Android Intent deserialization vulnerabilities with GSON parser:
+<a href="https://blog.oversecured.com/Exploiting-memory-corruption-vulnerabilities-on-Android/#insecure-use-of-json-parsers">Insecure use of JSON parsers</a>.
+</li>
+<li>
+RCE in Flexjson:
+<a href="https://codewhitesec.blogspot.com/2020/03/liferay-portal-json-vulns.html">Flexjson deserialization</a>.
+</li>
+<li>
+Jabsorb documentation on deserialization:
+<a href="https://github.com/Servoy/jabsorb/blob/master/src/org/jabsorb/">Jabsorb JSON Serializer</a>.
+</li>
+<li>
+JoddJson documentation on deserialization:
+<a href="https://json.jodd.org/parser">JoddJson Parser</a>.
+</li>
 </references>
 
 </qhelp>

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

@@ -40,6 +40,14 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
       ma.getArgument(0) = pred.asExpr() and
       ma.getQualifier() = succ.asExpr()
     )
+    or
+    exists(ConstructorCall cc |
+      cc.getConstructedType().getASupertype*().hasQualifiedName("org.json", "JSONObject") and
+      pred.asExpr() = cc.getAnArgument() and
+      succ.asExpr() = cc
+    )
+    or
+    isReflectiveClassIdentifierStep(pred, succ)
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -54,6 +62,28 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
       ma.getArgument(0) = node.asExpr() and
       exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1)))
     )
+    or
+    exists(MethodAccess ma |
+      (
+        ma.getMethod() instanceof FlexjsonDeserializeMethod or
+        ma.getMethod() instanceof JoddJsonParseMethod
+      ) and
+      node.asExpr() = ma.getAnArgument() and
+      (
+        ma.getArgument(1).getType() instanceof TypeClass and
+        not ma.getArgument(1).getType().getName() = ["Class<Object>", "Class<?>"]
+        or
+        exists(
+          MethodAccess dma // Specified class type
+        |
+          isSafeFlexjsonUseCall(dma) and
+          (
+            ma.getQualifier() = dma or // new flexjson.JSONDeserializer<Person>().use(null, Person.class).deserialize(json)
+            ma.getQualifier().(VarAccess).getVariable().getAnAccess() = dma.getQualifier()
+          )
+        )
+      )
+    )
   }
 }
 

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

@@ -0,0 +1,48 @@
+/**
+ * Provides classes for working with the Flexjson framework.
+ */
+
+import java
+
+/** The class `flexjson.JSONDeserializer`. */
+class FlexjsonDeserializer extends RefType {
+  FlexjsonDeserializer() { this.hasQualifiedName("flexjson", "JSONDeserializer") }
+}
+
+/** The class `flexjson.JSONSerializer`. */
+class FlexjsonSerializer extends RefType {
+  FlexjsonSerializer() { this.hasQualifiedName("flexjson", "JSONSerializer") }
+}
+
+/** 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 serialization method `serialize`. */
+class FlexjsonSerializeMethod extends Method {
+  FlexjsonSerializeMethod() {
+    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof
+      FlexjsonSerializer and
+    this.hasName(["serialize", "deepSerialize"])
+  }
+}
+
+/** The method `use` to configure allowed class type. */
+class DeserializerUseMethod extends Method {
+  DeserializerUseMethod() {
+    this.getDeclaringType().getSourceDeclaration().getASourceSupertype*() instanceof
+      FlexjsonDeserializer and
+    this.hasName("use")
+  }
+}

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

@@ -0,0 +1,26 @@
+/**
+ * Provides classes for working with the Jabsorb JSON-RPC ORB framework.
+ */
+
+import java
+
+/** The class `org.jabsorb.JSONSerializer`. */
+class JabsorbSerializer extends RefType {
+  JabsorbSerializer() { this.hasQualifiedName("org.jabsorb", "JSONSerializer") }
+}
+
+/** The deserialization method `unmarshall`. */
+class JabsorbUnmarshallMethod extends Method {
+  JabsorbUnmarshallMethod() {
+    this.getDeclaringType().getASupertype*() instanceof JabsorbSerializer and
+    this.getName() = "unmarshall"
+  }
+}
+
+/** The deserialization method `fromJSON`. */
+class JabsorbFromJsonMethod extends Method {
+  JabsorbFromJsonMethod() {
+    this.getDeclaringType().getASupertype*() instanceof JabsorbSerializer and
+    this.getName() = "fromJSON"
+  }
+}

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

@@ -0,0 +1,67 @@
+/**
+ * Provides classes and predicates for working with the JoddJson framework.
+ */
+
+import java
+
+/** The class `jodd.json.Parser`. */
+class JoddJsonParser extends RefType {
+  JoddJsonParser() { this.hasQualifiedName("jodd.json", "JsonParser") }
+}
+
+/** The class `jodd.json.JsonSerializer`. */
+class JoddJsonSerializer extends RefType {
+  JoddJsonSerializer() { this.hasQualifiedName("jodd.json", "JsonSerializer") }
+}
+
+/** The `parse*` deserialization method. */
+class JoddJsonParseMethod extends Method {
+  JoddJsonParseMethod() {
+    this.getDeclaringType() instanceof JoddJsonParser and
+    this.getName().matches("parse%")
+  }
+}
+
+/** The serialization method `serialize`. */
+class JoddJsonSerializeMethod extends Method {
+  JoddJsonSerializeMethod() {
+    this.getDeclaringType() instanceof JoddJsonSerializer and
+    this.hasName("serialize")
+  }
+}
+
+/** The `setClassMetadataName` method. */
+class SetClassMetadataNameMethod extends Method {
+  SetClassMetadataNameMethod() {
+    this.getDeclaringType() instanceof JoddJsonParser and
+    this.hasName("setClassMetadataName")
+  }
+}
+
+/** A call to `parser.withClassMetadata` method. */
+class WithClassMetadata extends MethodAccess {
+  WithClassMetadata() {
+    this.getMethod().getDeclaringType() instanceof JoddJsonParser and
+    this.getMethod().hasName("withClassMetadata")
+  }
+
+  /** Gets the constant value passed to this call. */
+  boolean getMode() { result = this.getArgument(0).(CompileTimeConstantExpr).getBooleanValue() }
+}
+
+/**
+ * Holds if there is a call to `parser.withClassMetadata` that explicitly enables
+ * class metadata.
+ */
+predicate enablesClassMetadata(WithClassMetadata wcm) { wcm.getMode() = true }
+
+/** A call to `parser.allowClass` method. */
+class SetWhitelistClasses extends MethodAccess {
+  SetWhitelistClasses() {
+    this.getMethod().getDeclaringType() instanceof JoddJsonParser and
+    this.getMethod().hasName("allowClass")
+  }
+
+  /** Gets the configured value. */
+  Expr getValue() { result = this.getArgument(0) }
+}

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

@@ -3,6 +3,7 @@
  */
 
 import java
+import semmle.code.java.dataflow.FlowSteps
 import semmle.code.xml.AndroidManifest
 
 /**
@@ -77,3 +78,28 @@ class AndroidContentResolver extends AndroidComponent {
     this.getASupertype*().hasQualifiedName("android.content", "ContentResolver")
   }
 }
+
+/** 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 getter on `android.os.BaseBundle` or `android.os.Bundle`. */
+class BundleGetterMethod extends Method, TaintPreservingCallable {
+  BundleGetterMethod() {
+    getDeclaringType().hasQualifiedName("android.os", ["BaseBundle", "Bundle"]) and
+    getName().matches("get%")
+  }
+
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
+}
+
+/** A reader method on `android.os.Parcel`. */
+class ParcelReaderMethod extends Method, TaintPreservingCallable {
+  ParcelReaderMethod() {
+    getDeclaringType().hasQualifiedName("android.os", "Parcel") and
+    getName().matches("read%")
+  }
+
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
+}

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

@@ -43,12 +43,9 @@ class IntentGetExtraMethod extends Method, TaintPreservingCallable {
   override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
 
-/** A getter on `android.os.BaseBundle` or `android.os.Bundle`. */
-class BundleGetterMethod extends Method, TaintPreservingCallable {
-  BundleGetterMethod() {
-    getDeclaringType().hasQualifiedName("android.os", ["BaseBundle", "Bundle"]) and
-    getName().matches("get%")
+class IntentGetParcelableExtraMethod extends Method {
+  IntentGetParcelableExtraMethod() {
+    hasName("getParcelableExtra") and
+    getDeclaringType() instanceof TypeIntent
   }
-
-  override predicate returnsTaintFrom(int arg) { arg = -1 }
 }

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

@@ -0,0 +1,47 @@
+/**
+ * Provides classes for working with the Gson framework.
+ */
+
+import java
+
+/** The class `com.google.gson.Gson`. */
+class Gson extends RefType {
+  Gson() { this.hasQualifiedName("com.google.gson", "Gson") }
+}
+
+/** The class `com.google.gson.GsonBuilder`. */
+class GsonBuilder extends RefType {
+  GsonBuilder() { this.hasQualifiedName("com.google.gson", "GsonBuilder") }
+}
+
+/** A method that registers class types in `GsonBuilder`. */
+class RegisterClassTypeMethod extends Method {
+  RegisterClassTypeMethod() {
+    this.getDeclaringType() instanceof GsonBuilder and
+    this.getName().matches("register%")
+  }
+}
+
+/** The `create` method of `GsonBuilder`. */
+class CreateGsonMethod extends Method {
+  CreateGsonMethod() {
+    this.getDeclaringType() instanceof GsonBuilder and
+    this.hasName("create")
+  }
+}
+
+/** The `fromJson` deserialization method. */
+class GsonDeserializeMethod extends Method {
+  GsonDeserializeMethod() {
+    this.getDeclaringType() instanceof Gson and
+    this.hasName("fromJson")
+  }
+}
+
+/** The `toJson` serialization method. */
+class GsonSerializeMethod extends Method {
+  GsonSerializeMethod() {
+    this.getDeclaringType() instanceof Gson and
+    this.hasName("toJson")
+  }
+}