Merge pull request #11713 from joefarebrother/sensitive-result-receiver

Java: Add query for leaking sensitive data through a ResultReceiver

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

@@ -0,0 +1,62 @@
+/** Definitions for the sensitive result receiver query. */
+
+import java
+import semmle.code.java.dataflow.TaintTracking2
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.SensitiveActions
+
+private class ResultReceiverSendCall extends MethodAccess {
+  ResultReceiverSendCall() {
+    this.getMethod()
+        .getASourceOverriddenMethod*()
+        .hasQualifiedName("android.os", "ResultReceiver", "send")
+  }
+
+  Expr getReceiver() { result = this.getQualifier() }
+
+  Expr getSentData() { result = this.getArgument(1) }
+}
+
+private class UntrustedResultReceiverConf extends TaintTracking2::Configuration {
+  UntrustedResultReceiverConf() { this = "UntrustedResultReceiverConf" }
+
+  override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node node) {
+    node.asExpr() = any(ResultReceiverSendCall c).getReceiver()
+  }
+}
+
+private predicate untrustedResultReceiverSend(DataFlow::Node src, ResultReceiverSendCall call) {
+  any(UntrustedResultReceiverConf c).hasFlow(src, DataFlow::exprNode(call.getReceiver()))
+}
+
+private class SensitiveResultReceiverConf extends TaintTracking::Configuration {
+  SensitiveResultReceiverConf() { this = "SensitiveResultReceiverConf" }
+
+  override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(ResultReceiverSendCall call |
+      untrustedResultReceiverSend(_, call) and
+      node.asExpr() = call.getSentData()
+    )
+  }
+
+  override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
+    super.allowImplicitRead(node, c)
+    or
+    this.isSink(node)
+  }
+}
+
+/** Holds if there is a path from sensitive data at `src` to a result receiver at `sink`, and the receiver was obtained from an untrusted source `recSrc`. */
+predicate sensitiveResultReceiver(
+  DataFlow::PathNode src, DataFlow::PathNode sink, DataFlow::Node recSrc
+) {
+  exists(ResultReceiverSendCall call, SensitiveResultReceiverConf conf |
+    conf.hasFlowPath(src, sink) and
+    sink.getNode().asExpr() = call.getSentData() and
+    untrustedResultReceiverSend(recSrc, call)
+  )
+}

java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.java

@@ -0,0 +1,8 @@
+// BAD: Sensitive data is sent to an untrusted result receiver 
+void bad(String password) {
+    Intent intent = getIntent();
+    ResultReceiver rec = intent.getParcelableExtra("Receiver");
+    Bundle b = new Bundle();
+    b.putCharSequence("pass", password);
+    rec.send(0, b); 
+}

java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.qhelp

@@ -0,0 +1,22 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>If a <code>ResultReceiver</code> is obtained from an untrusted source, such as an <code>Intent</code> received by an exported component, 
+    do not send it sensitive data. Otherwise, the information may be leaked to a malicious application.</p>
+  </overview>
+
+  <recommendation>
+    <p> 
+      Do not send sensitive data to an untrusted <code>ResultReceiver</code>.
+    </p>
+  </recommendation>
+
+  <example>
+    <p>In the following (bad) example, sensitive data is sent to an untrusted <code>ResultReceiver</code>. </p>
+    <sample src="SensitiveResultReceiver.java" />
+  </example>
+
+  <references>
+  </references>
+</qhelp>

java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql

@@ -0,0 +1,21 @@
+/**
+ * @name Leaking sensitive information through a ResultReceiver
+ * @description Sending sensitive data to a 'ResultReceiver' obtained from an untrusted source
+ *              can allow malicious actors access to your information.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 8.2
+ * @precision medium
+ * @id java/android/sensitive-result-receiver
+ * @tags security
+ *       external/cwe/cwe-927
+ */
+
+import java
+import semmle.code.java.security.SensitiveResultReceiverQuery
+import DataFlow::PathGraph
+
+from DataFlow::PathNode src, DataFlow::PathNode sink, DataFlow::Node recSrc
+where sensitiveResultReceiver(src, sink, recSrc)
+select sink, src, sink, "This $@ is sent to a ResultReceiver obtained from $@.", src,
+  "sensitive information", recSrc, "this untrusted source"

java/ql/src/change-notes/2023-01-12-sensitive-result-receiver.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `java/android/sensitive-result-receiver`, to find instances of sensitive data being leaked to an untrusted `ResultReceiver`.

java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.java

@@ -0,0 +1,15 @@
+import android.os.Bundle;
+import android.os.ResultReceiver;
+import android.content.Intent;
+
+class SensitiveResultReceiver {
+    <T> T source() { return null; }
+
+    void test1(String password) {
+        Intent intent = source();
+        ResultReceiver rec = intent.getParcelableExtra("hi");
+        Bundle b = new Bundle();
+        b.putCharSequence("pass", password);
+        rec.send(0, b); // $hasSensitiveResultReceiver
+    }
+}

java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.ql

@@ -0,0 +1,25 @@
+import java
+import TestUtilities.InlineExpectationsTest
+import semmle.code.java.security.SensitiveResultReceiverQuery
+
+class TestSource extends RemoteFlowSource {
+  TestSource() { this.asExpr().(MethodAccess).getMethod().hasName("source") }
+
+  override string getSourceType() { result = "test" }
+}
+
+class ResultReceiverTest extends InlineExpectationsTest {
+  ResultReceiverTest() { this = "ResultReceiverTest" }
+
+  override string getARelevantTag() { result = "hasSensitiveResultReceiver" }
+
+  override predicate hasActualResult(Location loc, string element, string tag, string value) {
+    exists(DataFlow::PathNode sink |
+      sensitiveResultReceiver(_, sink, _) and
+      element = sink.toString() and
+      loc = sink.getNode().getLocation() and
+      tag = "hasSensitiveResultReceiver" and
+      value = ""
+    )
+  }
+}