Merge pull request github#13358 from jorgectf/jorgectf/deserialization-lookahead

Java: Model `SerialKiller`

Author: atorralba

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

@@ -28,6 +28,20 @@ private class ObjectInputStreamReadObjectMethod extends Method {
   }
 }
 
+/**
+ * A type extending `ObjectInputStream` that makes it safe to deserialize untrusted data.
+ *
+ * * See https://commons.apache.org/proper/commons-io/javadocs/api-2.5/org/apache/commons/io/serialization/ValidatingObjectInputStream.html
+ * * See https://github.com/ikkisoft/SerialKiller
+ */
+private class SafeObjectInputStreamType extends RefType {
+  SafeObjectInputStreamType() {
+    this.getASourceSupertype*()
+        .hasQualifiedName("org.apache.commons.io.serialization", "ValidatingObjectInputStream") or
+    this.getASourceSupertype*().hasQualifiedName("org.nibblesec.tools", "SerialKiller")
+  }
+}
+
 private class XmlDecoderReadObjectMethod extends Method {
   XmlDecoderReadObjectMethod() {
     this.getDeclaringType().hasQualifiedName("java.beans", "XMLDecoder") and
@@ -135,9 +149,7 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
     sink = ma.getQualifier() and
     not exists(DataFlow::ExprNode node |
       node.getExpr() = sink and
-      node.getTypeBound()
-          .(RefType)
-          .hasQualifiedName("org.apache.commons.io.serialization", "ValidatingObjectInputStream")
+      node.getTypeBound() instanceof SafeObjectInputStreamType
     )
     or
     m instanceof XmlDecoderReadObjectMethod and

java/ql/src/change-notes/2023-06-02-unsafe-deserialization-serialkiller.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `java/unsafe-deserialization` has been updated to take into account `SerialKiller`, a library used to prevent deserialization of arbitrary classes.

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

@@ -7,6 +7,7 @@
 import org.yaml.snakeyaml.constructor.SafeConstructor;
 import org.yaml.snakeyaml.constructor.Constructor;
 import org.yaml.snakeyaml.Yaml;
+import org.nibblesec.tools.SerialKiller;
 
 public class A {
   public Object deserialize1(Socket sock) throws java.io.IOException, ClassNotFoundException {
@@ -21,6 +22,12 @@ public Object deserialize2(Socket sock) throws java.io.IOException, ClassNotFoun
     return in.readUnshared(); // $unsafeDeserialization
   }
 
+  public Object deserializeWithSerialKiller(Socket sock) throws java.io.IOException, ClassNotFoundException {
+    InputStream inputStream = sock.getInputStream();
+    ObjectInputStream in = new SerialKiller(inputStream, "/etc/serialkiller.conf");
+    return in.readUnshared(); // OK
+  }
+
   public Object deserialize3(Socket sock) throws java.io.IOException {
     InputStream inputStream = sock.getInputStream();
     XMLDecoder d = new XMLDecoder(inputStream);

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:${testdir}/../../../stubs/flexjson-2.1:${testdir}/../../../stubs/gson-2.8.6:${testdir}/../../../stubs/google-android-9.0.0
+//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:${testdir}/../../../stubs/gson-2.8.6:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/serialkiller-4.0.0