Java: Delayed unsafe deserialization

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

@@ -138,6 +138,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.Hibernate
   private import semmle.code.java.frameworks.jOOQ
   private import semmle.code.java.frameworks.spring.SpringHttp
+  private import semmle.code.java.frameworks.RabbitMQ
 }
 
 private predicate sourceModelCsv(string row) {

java/ql/lib/semmle/code/java/dataflow/FlowSteps.qll

@@ -21,6 +21,7 @@ private module Frameworks {
   private import semmle.code.java.frameworks.guava.Guava
   private import semmle.code.java.frameworks.apache.Lang
   private import semmle.code.java.frameworks.ApacheHttp
+  private import semmle.code.java.frameworks.RabbitMQ
 }
 
 /**

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

@@ -13,6 +13,10 @@ private class JavaIoSummaryCsv extends SummaryModelCsv {
         "java.io;Writer;true;write;;;Argument[0];Argument[-1];taint",
         "java.io;Writer;true;toString;;;Argument[-1];ReturnValue;taint",
         "java.io;CharArrayWriter;true;toCharArray;;;Argument[-1];ReturnValue;taint",
+        "java.io;ObjectInput;true;read;;;Argument[-1];Argument[0];taint",
+        "java.io;DataInput;true;readFully;;;Argument[-1];Argument[0];taint",
+        "java.io;DataInput;true;readLine;();;Argument[-1];Argument[0];taint",
+        "java.io;DataInput;true;readUTF;();;Argument[-1];Argument[0];taint",
         "java.nio.channels;ReadableByteChannel;true;read;(ByteBuffer);;Argument[-1];Argument[0];taint",
         "java.nio.channels;Channels;false;newChannel;(InputStream);;Argument[0];ReturnValue;taint"
       ]

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

@@ -0,0 +1,58 @@
+/**
+ * Provides classes and predicates related to RabbitMQ.
+ */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+
+/**
+ * Defines remote sources in RabbitMQ.
+ */
+private class RabbitMQSource extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        // soruces for RabbitMQ 4.x
+        "com.rabbitmq.client;Command;true;getContentHeader;();;ReturnValue;remote",
+        "com.rabbitmq.client;Command;true;getContentBody;();;ReturnValue;remote",
+        "com.rabbitmq.client;Consumer;true;handleDelivery;(String,Envelope,BasicProperties,byte[]);;Parameter[3];remote",
+        "com.rabbitmq.client;QueueingConsumer;true;nextDelivery;;;ReturnValue;remote",
+        "com.rabbitmq.client;RpcServer;true;handleCall;(Delivery,BasicProperties);;Parameter[0];remote",
+        "com.rabbitmq.client;RpcServer;true;handleCall;(BasicProperties,byte[],BasicProperties);;Parameter[1];remote",
+        "com.rabbitmq.client;RpcServer;true;handleCall;(byte[],BasicProperties);;Parameter[0];remote",
+        "com.rabbitmq.client;RpcServer;true;preprocessReplyProperties;(Delivery,Builder);;Parameter[0];remote",
+        "com.rabbitmq.client;RpcServer;true;postprocessReplyProperties;(Delivery,Builder);;Parameter[0];remote",
+        "com.rabbitmq.client;RpcServer;true;handleCast;(Delivery);;Parameter[0];remote",
+        "com.rabbitmq.client;RpcServer;true;handleCast;(BasicProperties,byte[]);;Parameter[1];remote",
+        "com.rabbitmq.client;RpcServer;true;handleCast;(byte[]);;Parameter[0];remote",
+        "com.rabbitmq.client;StringRpcServer;true;handleStringCall;;;Parameter[0];remote",
+        "com.rabbitmq.client;RpcClient;true;doCall;;;ReturnValue;remote",
+        "com.rabbitmq.client;RpcClient;true;primitiveCall;;;ReturnValue;remote",
+        "com.rabbitmq.client;RpcClient;true;responseCall;;;ReturnValue;remote",
+        "com.rabbitmq.client;RpcClient;true;stringCall;(String);;ReturnValue;remote",
+        "com.rabbitmq.client;RpcClient;true;mapCall;;;ReturnValue;remote",
+        "com.rabbitmq.client.impl;Frame;true;getInputStream;();;ReturnValue;remote",
+        "com.rabbitmq.client.impl;Frame;true;getPayload;();;ReturnValue;remote",
+        "com.rabbitmq.client.impl;FrameHandler;true;readFrame;();;ReturnValue;remote",
+      ]
+  }
+}
+
+/**
+ * Defines flow steps in RabbitMQ.
+ */
+private class RabbitMQSummaryCsv extends SummaryModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        // flow steps for RabbitMQ 4.x
+        "com.rabbitmq.client;GetResponse;true;GetResponse;;;Argument[2];Argument[-1];taint",
+        "com.rabbitmq.client;GetResponse;true;getBody;();;Argument[-1];ReturnValue;taint",
+        "com.rabbitmq.client;RpcClient$Response;true;getBody;();;Argument[-1];ReturnValue;taint",
+        "com.rabbitmq.client;QueueingConsumer$Delivery;true;getBody;();;Argument[-1];ReturnValue;taint",
+        "com.rabbitmq.client.impl;Frame;false;fromBodyFragment;(int,byte[],int,int);;Argument[1];ReturnValue;taint",
+        "com.rabbitmq.client.impl;Frame;false;readFrom;(DataInputStream);;Argument[0];ReturnValue;taint",
+        "com.rabbitmq.client.impl;Frame;true;writeTo;(DataOutputStream);;Argument[-1];Argument[0];taint",
+      ]
+  }
+}

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

@@ -256,6 +256,8 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration {
     createJacksonTreeNodeStep(pred, succ)
     or
     intentFlowsToParcel(pred, succ)
+    or
+    byteArrayFieldFlowStep(pred, succ)
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -519,3 +521,14 @@ private predicate joddJsonParserConfiguredUnsafely(Expr parserExpr) {
     not config.hasFlow(getASafelyConfiguredParser(), parser)
   )
 }
+
+/**
+ * Holds if `fromNode` to `toNode` is a dataflow step that stores data to a byte array field.
+ */
+private predicate byteArrayFieldFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
+  exists(FieldAccess access, Field field | field = access.getField() |
+    field.getType().hasName("byte[]") and
+    access = fromNode.asExpr() and
+    field.getAnAccess() = toNode.asExpr()
+  )
+}

java/ql/src/change-notes/2022--03-19-unsafe-deserialization.md

@@ -0,0 +1,7 @@
+---
+category: majorAnalysis
+---
+ * Updated `java/unsafe-deserialization` query to detect delayed unsafe deserialization.
+ * Added flow steps for `java.io.DataInput` and `java.io.ObjectInput` implementations.
+ * Added flow sources and steps for RabbitMQ.
+

java/ql/test/library-tests/rabbitmq/FlowTest.ql

@@ -0,0 +1,20 @@
+import java
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineFlowTest
+
+class Conf extends TaintTracking::Configuration {
+  Conf() { this = "qltest:frameworks:rabbitmq" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(MethodAccess ma | ma.getMethod().hasName("sink") | node.asExpr() = ma.getAnArgument())
+  }
+}
+
+class HasFlowTest extends InlineFlowTest {
+  override DataFlow::Configuration getValueFlowConfig() { none() }
+
+  override DataFlow::Configuration getTaintFlowConfig() { result = any(Conf c) }
+}

java/ql/test/library-tests/rabbitmq/SourceTest.ql

@@ -0,0 +1,19 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineExpectationsTest
+
+class SourceTest extends InlineExpectationsTest {
+  SourceTest() { this = "SourceTest" }
+
+  override string getARelevantTag() { result = "source" }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "source" and
+    exists(RemoteFlowSource source |
+      not source.asParameter().getCallable().getDeclaringType().hasName("DefaultConsumer") and
+      source.getLocation() = location and
+      element = source.toString() and
+      value = ""
+    )
+  }
+}

java/ql/test/library-tests/rabbitmq/Test.java

@@ -0,0 +1,34 @@
+import com.rabbitmq.client.DefaultConsumer;
+import com.rabbitmq.client.Channel;
+import com.rabbitmq.client.AMQP;
+import com.rabbitmq.client.Envelope;
+import com.rabbitmq.client.QueueingConsumer;
+
+public class Test {
+
+    public void defaultConsumerTest(Channel channel) {
+        DefaultConsumer consumer = new DefaultConsumer(channel) {
+
+            @Override
+            public void handleDelivery(
+                    String consumerTag, Envelope envelope, AMQP.BasicProperties properties, 
+                    byte[] body) { // $source
+
+                sink(body); // $hasTaintFlow
+            }
+        };
+    }
+
+    public void queueingConsumerTest(QueueingConsumer consumer) {
+        while (true) {
+            QueueingConsumer.Delivery delivery = consumer.nextDelivery(); // $source
+            sink(delivery.getBody()); // $hasTaintFlow
+            delivery = consumer.nextDelivery(42); // $source
+            sink(delivery.getBody()); // $hasTaintFlow
+        }
+    }
+
+    private void sink(byte[] data) {
+
+    }
+}

java/ql/test/library-tests/rabbitmq/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/rabbitmq-4.12.0

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

@@ -0,0 +1,30 @@
+import java.io.ByteArrayInputStream;
+import java.io.DataInput;
+import java.io.DataInputStream;
+import java.io.ObjectInputStream;
+import java.net.Socket;
+
+public class DelayedDeserialization {
+
+    public void test(Socket socket) throws Exception {
+        DataInputStream in = new DataInputStream(socket.getInputStream());
+        ObjectHolder holder = new ObjectHolder();
+        holder.init(in);
+        holder.getObject();
+    }
+}
+
+class ObjectHolder {
+
+    private final byte[] buffer = new byte[1024];
+
+    public void init(DataInput input) throws Exception {
+        input.readFully(buffer, 0, buffer.length);
+    }
+
+    public Object getObject() throws Exception {
+        ByteArrayInputStream bais = new ByteArrayInputStream(buffer);
+        ObjectInputStream ois = new ObjectInputStream(bais);
+        return ois.readObject(); // $unsafeDeserialization
+    }
+}