Use MaD models for unsafe deserialization sinks when possible

Many of the unsafe deserialization sinks have to stay defined in QL
because they have custom logic that cannot be expressed in MaD models.

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

@@ -1,20 +1,28 @@
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * Provides classes and predicates for working with the Castor framework.
  */
 overlay[local?]
-module;
+deprecated module;
 
 import java
 
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * The class `org.exolab.castor.xml.Unmarshaller`.
  */
-class CastorUnmarshaller extends RefType {
+deprecated class CastorUnmarshaller extends RefType {
   CastorUnmarshaller() { this.hasQualifiedName("org.exolab.castor.xml", "Unmarshaller") }
 }
 
-/** A method with the name `unmarshal` declared in `org.exolab.castor.xml.Unmarshaller`. */
-class CastorUnmarshalMethod extends Method {
+/**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
+ * A method with the name `unmarshal` declared in `org.exolab.castor.xml.Unmarshaller`.
+ */
+deprecated class CastorUnmarshalMethod extends Method {
   CastorUnmarshalMethod() {
     this.getDeclaringType() instanceof CastorUnmarshaller and
     this.getName() = "unmarshal"

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

@@ -17,10 +17,12 @@ class UnsafeHessianInput extends RefType {
 }
 
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * A AbstractHessianInput or Hessian2StreamingInput subclass readObject method.
  * This is either `AbstractHessianInput.readObject` or `Hessian2StreamingInput.readObject`.
  */
-class UnsafeHessianInputReadObjectMethod extends Method {
+deprecated class UnsafeHessianInputReadObjectMethod extends Method {
   UnsafeHessianInputReadObjectMethod() {
     this.getDeclaringType().getAnAncestor() instanceof UnsafeHessianInput and
     this.getName() = "readObject"
@@ -34,8 +36,12 @@ class BurlapInput extends RefType {
   BurlapInput() { this.hasQualifiedName("com.caucho.burlap.io", "BurlapInput") }
 }
 
-/** A method with the name `readObject` declared in `com.caucho.burlap.io.BurlapInput`. */
-class BurlapInputReadObjectMethod extends Method {
+/**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
+ *  A method with the name `readObject` declared in `com.caucho.burlap.io.BurlapInput`.
+ */
+deprecated class BurlapInputReadObjectMethod extends Method {
   BurlapInputReadObjectMethod() {
     this.getDeclaringType() instanceof BurlapInput and
     this.getName() = "readObject"

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

@@ -1,22 +1,28 @@
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * Provides classes and predicates for working with the JYaml framework.
  */
 overlay[local?]
-module;
+deprecated module;
 
 import java
 
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * The class `org.ho.yaml.Yaml` or `org.ho.yaml.YamlConfig`.
  */
-class JYamlLoader extends RefType {
+deprecated class JYamlLoader extends RefType {
   JYamlLoader() { this.hasQualifiedName("org.ho.yaml", ["Yaml", "YamlConfig"]) }
 }
 
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * A JYaml unsafe load method, declared on either `Yaml` or `YamlConfig`.
  */
-class JYamlLoaderUnsafeLoadMethod extends Method {
+deprecated class JYamlLoaderUnsafeLoadMethod extends Method {
   JYamlLoaderUnsafeLoadMethod() {
     this.getDeclaringType() instanceof JYamlLoader and
     this.getName() in ["load", "loadType", "loadStream", "loadStreamOfType"]

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

@@ -19,8 +19,12 @@ class JabsorbUnmarshallMethod extends Method {
   }
 }
 
-/** The deserialization method `fromJSON`. */
-class JabsorbFromJsonMethod extends Method {
+/**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
+ * The deserialization method `fromJSON`.
+ */
+deprecated class JabsorbFromJsonMethod extends Method {
   JabsorbFromJsonMethod() {
     this.getDeclaringType().getAnAncestor() instanceof JabsorbSerializer and
     this.getName() = "fromJSON"

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

@@ -13,8 +13,12 @@ class YamlBeansReader extends RefType {
   YamlBeansReader() { this.hasQualifiedName("com.esotericsoftware.yamlbeans", "YamlReader") }
 }
 
-/** A method with the name `read` declared in `com.esotericsoftware.yamlbeans.YamlReader`. */
-class YamlBeansReaderReadMethod extends Method {
+/**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
+ * A method with the name `read` declared in `com.esotericsoftware.yamlbeans.YamlReader`.
+ */
+deprecated class YamlBeansReaderReadMethod extends Method {
   YamlBeansReaderReadMethod() {
     this.getDeclaringType() instanceof YamlBeansReader and
     this.getName() = "read"

java/ql/lib/semmle/code/java/frameworks/apache/Lang.qll

@@ -16,10 +16,12 @@ class TypeApacheRandomStringUtils extends Class {
 }
 
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * The method `deserialize` in either `org.apache.commons.lang.SerializationUtils`
  * or `org.apache.commons.lang3.SerializationUtils`.
  */
-class MethodApacheSerializationUtilsDeserialize extends Method {
+deprecated class MethodApacheSerializationUtilsDeserialize extends Method {
   MethodApacheSerializationUtilsDeserialize() {
     this.getDeclaringType()
         .hasQualifiedName(["org.apache.commons.lang", "org.apache.commons.lang3"],

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

@@ -3,17 +3,16 @@
  */
 
 import semmle.code.java.dataflow.FlowSources
+private import semmle.code.java.dataflow.ExternalFlow
 private import semmle.code.java.dataflow.FlowSinks
 private import semmle.code.java.dispatch.VirtualDispatch
 private import semmle.code.java.frameworks.Kryo
 private import semmle.code.java.frameworks.XStream
 private import semmle.code.java.frameworks.SnakeYaml
 private import semmle.code.java.frameworks.FastJson
-private import semmle.code.java.frameworks.JYaml
 private import semmle.code.java.frameworks.JsonIo
 private import semmle.code.java.frameworks.YamlBeans
 private import semmle.code.java.frameworks.HessianBurlap
-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.Jms
@@ -149,8 +148,15 @@ private module SafeKryoConfig implements DataFlow::ConfigSig {
 
 private module SafeKryoFlow = DataFlow::Global<SafeKryoConfig>;
 
+private class DefaultUnsafeDeserializationSink extends DataFlow::Node {
+  DefaultUnsafeDeserializationSink() { sinkNode(this, "unsafe-deserialization") }
+}
+
 /**
  * Holds if `ma` is a call that deserializes data from `sink`.
+ *
+ * Note that this does not include deserialization methods that have been
+ * specified using models-as-data.
  */
 predicate unsafeDeserialization(MethodCall ma, Expr sink) {
   exists(Method m | m = ma.getMethod() |
@@ -162,9 +168,6 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
     sink = ma.getQualifier() and
     not DataFlow::exprNode(sink).getTypeBound() instanceof SafeObjectInputStreamType
     or
-    m instanceof XmlDecoderReadObjectMethod and
-    sink = ma.getQualifier()
-    or
     m instanceof XStreamReadObjectMethod and
     sink = ma.getAnArgument() and
     not SafeXStreamFlow::flowToExpr(ma.getQualifier())
@@ -173,9 +176,6 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
     sink = ma.getAnArgument() and
     not SafeKryoFlow::flowToExpr(ma.getQualifier())
     or
-    m instanceof MethodApacheSerializationUtilsDeserialize and
-    sink = ma.getArgument(0)
-    or
     ma instanceof UnsafeSnakeYamlParse and
     sink = ma.getArgument(0)
     or
@@ -183,23 +183,6 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
     not fastJsonLooksSafe() and
     sink = ma.getArgument(0)
     or
-    ma.getMethod() instanceof JYamlLoaderUnsafeLoadMethod and
-    sink = ma.getArgument(0)
-    or
-    ma.getMethod() instanceof JsonIoJsonToJavaMethod and
-    sink = ma.getArgument(0)
-    or
-    ma.getMethod() instanceof JsonIoReadObjectMethod and
-    sink = ma.getQualifier()
-    or
-    ma.getMethod() instanceof YamlBeansReaderReadMethod and sink = ma.getQualifier()
-    or
-    ma.getMethod() instanceof UnsafeHessianInputReadObjectMethod and sink = ma.getQualifier()
-    or
-    ma.getMethod() instanceof CastorUnmarshalMethod and sink = ma.getAnArgument()
-    or
-    ma.getMethod() instanceof BurlapInputReadObjectMethod and sink = ma.getQualifier()
-    or
     ma.getMethod() instanceof ObjectMapperReadMethod and
     sink = ma.getArgument(0) and
     (
@@ -215,9 +198,6 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
     sink = ma.getArgument(2) and
     UnsafeTypeFlow::flowToExpr(ma.getArgument(1))
     or
-    m instanceof JabsorbFromJsonMethod and
-    sink = ma.getArgument(0)
-    or
     m instanceof JoddJsonParseMethod and
     sink = ma.getArgument(0) and
     (
@@ -244,10 +224,17 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
 
 /** A sink for unsafe deserialization. */
 class UnsafeDeserializationSink extends ApiSinkNode, DataFlow::ExprNode {
-  UnsafeDeserializationSink() { unsafeDeserialization(_, this.getExpr()) }
+  MethodCall mc;
+
+  UnsafeDeserializationSink() {
+    unsafeDeserialization(mc, this.getExpr())
+    or
+    this instanceof DefaultUnsafeDeserializationSink and
+    this.getExpr() = [mc.getQualifier(), mc.getAnArgument()]
+  }
 
   /** Gets a call that triggers unsafe deserialization. */
-  MethodCall getMethodCall() { unsafeDeserialization(result, this.getExpr()) }
+  MethodCall getMethodCall() { result = mc }
 }
 
 /** Holds if `node` is a sanitizer for unsafe deserialization */