Merge pull request github#4603 from pwntester/new_deser_sink

New UnsafeDeserialization sink and improvements to SnakeYaml sink

Author: aschackmull

java/change-notes/2020-11-04-commonslang-unsafe-deserialization-sinks.md

@@ -0,0 +1,3 @@
+lgtm,codescanning
+* The query "Deserialization of user-controlled data" (`java/unsafe-deserialization`) has been improved to recognize unsafe Apache Commons Lang(3) methods.
+* The SnakeYAML Unsafe Deserialization sink has been improved to recognize `compose` and `composeAll` unsafe methods.

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

@@ -39,7 +39,7 @@ class SafeSnakeYamlConstruction extends ClassInstanceExpr {
  * The class `org.yaml.snakeyaml.Yaml`.
  */
 class Yaml extends RefType {
-  Yaml() { this.hasQualifiedName("org.yaml.snakeyaml", "Yaml") }
+  Yaml() { this.getASupertype*().hasQualifiedName("org.yaml.snakeyaml", "Yaml") }
 }
 
 private class SafeYamlConstructionFlowConfig extends DataFlow2::Configuration {
@@ -71,7 +71,7 @@ private class SnakeYamlParse extends MethodAccess {
   SnakeYamlParse() {
     exists(Method m |
       m.getDeclaringType() instanceof Yaml and
-      (m.hasName("load") or m.hasName("loadAll") or m.hasName("loadAs") or m.hasName("parse")) and
+      m.hasName(["compose", "composeAll", "load", "loadAll", "loadAs", "parse"]) and
       m = this.getMethod()
     )
   }

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

@@ -10,3 +10,18 @@ class TypeApacheRandomStringUtils extends Class {
     hasQualifiedName("org.apache.commons.lang3", "RandomStringUtils")
   }
 }
+
+/*--- Methods ---*/
+/**
+ * The method `deserialize` in either `org.apache.commons.lang.SerializationUtils`
+ * or `org.apache.commons.lang3.SerializationUtils`.
+ */
+class MethodApacheSerializationUtilsDeserialize extends Method {
+  MethodApacheSerializationUtilsDeserialize() {
+    (
+      this.getDeclaringType().hasQualifiedName("org.apache.commons.lang", "SerializationUtils") or
+      this.getDeclaringType().hasQualifiedName("org.apache.commons.lang3", "SerializationUtils")
+    ) and
+    this.hasName("deserialize")
+  }
+}

java/ql/src/semmle/code/java/security/UnsafeDeserialization.qll

@@ -1,6 +1,7 @@
 import semmle.code.java.frameworks.Kryo
 import semmle.code.java.frameworks.XStream
 import semmle.code.java.frameworks.SnakeYaml
+import semmle.code.java.frameworks.apache.Lang
 
 class ObjectInputStreamReadObjectMethod extends Method {
   ObjectInputStreamReadObjectMethod() {
@@ -71,6 +72,9 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) {
     sink = ma.getAnArgument() and
     not exists(SafeKryo sk | sk.hasFlowToExpr(ma.getQualifier()))
     or
+    m instanceof MethodApacheSerializationUtilsDeserialize and
+    sink = ma.getArgument(0)
+    or
     ma instanceof UnsafeSnakeYamlParse and
     sink = ma.getArgument(0)
   )