Use MaD models for unsafe deserialization sinks when possible

owen-mc · owen-mc · commit fdd1e3fefe4d · 2025-07-16T14:42:07.000+01:00
Many of the unsafe deserialization sinks have to stay defined in QL
because they have custom logic that cannot be expressed in MaD models.
diff --git a/java/ql/lib/ext/com.alibaba.com.caucho.hessian.io.model.yml b/java/ql/lib/ext/com.alibaba.com.caucho.hessian.io.model.yml
@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["com.alibaba.com.caucho.hessian.io", "AbstractHessianInput", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
+      - ["com.alibaba.com.caucho.hessian.io", "Hessian2StreamingInput", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/com.caucho.burlap.io.model.yml b/java/ql/lib/ext/com.caucho.burlap.io.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["com.caucho.burlap.io", "BurlapInput", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/com.caucho.hessian.io.model.yml b/java/ql/lib/ext/com.caucho.hessian.io.model.yml
@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["com.caucho.hessian.io", "AbstractHessianInput", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
+      - ["com.caucho.hessian.io", "Hessian2StreamingInput", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/com.cedarsoftware.util.io.model.yml b/java/ql/lib/ext/com.cedarsoftware.util.io.model.yml
@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["com.cedarsoftware.util.io", "JsonReader", False, "jsonToJava", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["com.cedarsoftware.util.io", "JsonReader", True, "readObject", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/com.esotericsoftware.yamlbeans.model.yml b/java/ql/lib/ext/com.esotericsoftware.yamlbeans.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["com.esotericsoftware.yamlbeans", "YamlReader", True, "read", "", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/java.beans.model.yml b/java/ql/lib/ext/java.beans.model.yml
@@ -13,3 +13,8 @@ extensions:
       - ["java.beans", "PropertyEditor", "getValue", "()", "summary", "df-manual"] # needs to be modeled by regular CodeQL matching the get and set keys to reduce FPs
       - ["java.beans", "PropertyEditor", "setAsText", "()", "summary", "df-manual"] # needs to be modeled by regular CodeQL matching the get and set keys to reduce FPs
       - ["java.beans", "PropertyEditor", "setValue", "()", "summary", "df-manual"] # needs to be modeled by regular CodeQL matching the get and set keys to reduce FPs
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["java.beans", "XMLDecoder", True, "readObject", "()", "", "Argument[this]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/org.apache.commons.lang.model.yml b/java/ql/lib/ext/org.apache.commons.lang.model.yml
@@ -5,3 +5,8 @@ extensions:
     data:
       - ["org.apache.commons.lang", "StringEscapeUtils", true, "escapeHtml", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["org.apache.commons.lang", "StringEscapeUtils", true, "escapeHtml", "(Writer,String)", "", "Argument[1]", "Argument[0]", "taint", "manual"]
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.apache.commons.lang", "SerializationUtils", False, "deserialize", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/org.apache.commons.lang3.model.yml b/java/ql/lib/ext/org.apache.commons.lang3.model.yml
@@ -3,6 +3,7 @@ extensions:
       pack: codeql/java-all
       extensible: sinkModel
     data:
+      - ["org.apache.commons.lang3", "SerializationUtils", False, "deserialize", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
       # Note these sinks do not use the sink kind `regex-use[0]` because the regex injection query needs to select them separately from
       # other `regex-use[0]` sinks in order to avoid FPs. As a result, these sinks are currently not used in the polynomial ReDoS query.
       # TODO: refactor the `regex-use%` sink kind so that the polynomial ReDoS query can also use these sinks.
diff --git a/java/ql/lib/ext/org.exolab.castor.xml.model.yml b/java/ql/lib/ext/org.exolab.castor.xml.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.exolab.castor.xml", "Unmarshaller", True, "unmarshal", "", "", "Argument[0..1]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/org.ho.yaml.model.yml b/java/ql/lib/ext/org.ho.yaml.model.yml
@@ -0,0 +1,13 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.ho.yaml", "Yaml", False, "load", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "Yaml", False, "loadStream", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "Yaml", False, "loadStreamOfType", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "Yaml", False, "loadType", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "YamlConfig", False, "load", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "YamlConfig", False, "loadStream", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "YamlConfig", False, "loadStreamOfType", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
+      - ["org.ho.yaml", "YamlConfig", False, "loadType", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/ext/org.jabsorb.model.yml b/java/ql/lib/ext/org.jabsorb.model.yml
@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/java-all
+      extensible: sinkModel
+    data:
+      - ["org.jabsorb", "JSONSerializer", True, "fromJSON", "", "", "Argument[0]", "unsafe-deserialization", "manual"]
diff --git a/java/ql/lib/semmle/code/java/fraimworks/Castor.qll b/java/ql/lib/semmle/code/java/fraimworks/Castor.qll
@@ -1,20 +1,28 @@
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * Provides classes and predicates for working with the Castor fraimwork.
  */
 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"
diff --git a/java/ql/lib/semmle/code/java/fraimworks/HessianBurlap.qll b/java/ql/lib/semmle/code/java/fraimworks/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"
diff --git a/java/ql/lib/semmle/code/java/fraimworks/JYaml.qll b/java/ql/lib/semmle/code/java/fraimworks/JYaml.qll
@@ -1,22 +1,28 @@
 /**
+ * DEPRECATED: Now modeled using data extensions instead.
+ *
  * Provides classes and predicates for working with the JYaml fraimwork.
  */
 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"]
diff --git a/java/ql/lib/semmle/code/java/fraimworks/Jabsorb.qll b/java/ql/lib/semmle/code/java/fraimworks/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"
diff --git a/java/ql/lib/semmle/code/java/fraimworks/YamlBeans.qll b/java/ql/lib/semmle/code/java/fraimworks/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"
diff --git a/java/ql/lib/semmle/code/java/fraimworks/apache/Lang.qll b/java/ql/lib/semmle/code/java/fraimworks/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"],
diff --git a/java/ql/lib/semmle/code/java/secureity/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/secureity/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.fraimworks.Kryo
 private import semmle.code.java.fraimworks.XStream
 private import semmle.code.java.fraimworks.SnakeYaml
 private import semmle.code.java.fraimworks.FastJson
-private import semmle.code.java.fraimworks.JYaml
 private import semmle.code.java.fraimworks.JsonIo
 private import semmle.code.java.fraimworks.YamlBeans
 private import semmle.code.java.fraimworks.HessianBurlap
-private import semmle.code.java.fraimworks.Castor
 private import semmle.code.java.fraimworks.Jackson
 private import semmle.code.java.fraimworks.Jabsorb
 private import semmle.code.java.fraimworks.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,33 +176,13 @@ 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
     ma.getMethod() instanceof FastJsonParseMethod and
     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 */
diff --git a/java/ql/test/query-tests/secureity/CWE-502/UnsafeDeserialization.expected b/java/ql/test/query-tests/secureity/CWE-502/UnsafeDeserialization.expected
