Merge pull request #14610 from atorralba/atorralba/java/jms-deserialization

Java: Add JMS sink to java/unsafe-deserialization

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

@@ -0,0 +1,10 @@
+/** Provides definitions for working with the JMS library. */
+
+import java
+
+/** The method `ObjectMessage.getObject`. */
+class ObjectMessageGetObjectMethod extends Method {
+  ObjectMessageGetObjectMethod() {
+    this.hasQualifiedName(["javax", "jakarta"] + ".jms", "ObjectMessage", "getObject")
+  }
+}

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

@@ -4,6 +4,7 @@
 
 import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.TaintTracking2
+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
@@ -15,6 +16,7 @@ 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
 private import semmle.code.java.frameworks.JoddJson
 private import semmle.code.java.frameworks.Flexjson
 private import semmle.code.java.frameworks.google.Gson
@@ -224,6 +226,11 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) {
     m instanceof GsonDeserializeMethod and
     sink = ma.getArgument(0) and
     UnsafeTypeFlow::flowToExpr(ma.getArgument(1))
+    or
+    m.getASourceOverriddenMethod*() instanceof ObjectMessageGetObjectMethod and
+    sink = ma.getQualifier().getUnderlyingExpr() and
+    // If we can see an implementation, we trust dataflow to find a path to the other sinks instead
+    not exists(viableCallable(ma))
   )
 }
 

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

@@ -15,7 +15,7 @@ may have unforeseen effects, such as the execution of arbitrary code.
 <p>
 There are many different serialization frameworks.  This query currently
 supports Kryo, XmlDecoder, XStream, SnakeYaml, JYaml, JsonIO, YAMLBeans, HessianBurlap, Castor, Burlap,
-Jackson, Jabsorb, Jodd JSON, Flexjson, Gson and Java IO serialization through
+Jackson, Jabsorb, Jodd JSON, Flexjson, Gson, JMS, and Java IO serialization through
 <code>ObjectInputStream</code>/<code>ObjectOutputStream</code>.
 </p>
 </overview>
@@ -74,6 +74,12 @@ Recommendations specific to particular frameworks supported by this query:
         <li><b>Recommendation</b>: Do not use with untrusted user input.</li>
     </ul>
 <p></p>
+<p><b>ObjectMesssage</b> - <code>Java EE/Jakarta EE</code></p>
+    <ul>
+        <li><b>Secure by Default</b>: Depends on the JMS implementation.</li>
+        <li><b>Recommendation</b>: Do not use with untrusted user input.</li>
+    </ul>
+<p></p>
 </recommendation>
 
 <example>
@@ -158,6 +164,10 @@ RCE in Flexjson:
 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>
+Research by Matthias Kaiser:
+<a href="https://www.blackhat.com/docs/us-16/materials/us-16-Kaiser-Pwning-Your-Java-Messaging-With-Deserialization-Vulnerabilities.pdf">Pwning Your Java Messaging With Deserialization Vulnerabilities</a>.
+</li>
 </references>
 
 </qhelp>

java/ql/src/change-notes/2023-10-26-jms-unsafe-deserialization.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `java/unsafe-deserialization` has been improved to detect insecure calls to `ObjectMessage.getObject` in JMS.

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

@@ -0,0 +1,9 @@
+import javax.jms.Message;
+import javax.jms.MessageListener;
+import javax.jms.ObjectMessage;
+
+public class ObjectMessageTest implements MessageListener {
+    public void onMessage(Message message) {
+        ((ObjectMessage) message).getObject(); // $ unsafeDeserialization
+    }
+}

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:${testdir}/../../../stubs/serialkiller-4.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:${testdir}/../../../stubs/jms-api-1