diff --git a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll new file mode 100644 index 000000000000..0679f66b5751 --- /dev/null +++ b/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) + ) +} diff --git a/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.java b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.java new file mode 100644 index 000000000000..0caf4cf3595b --- /dev/null +++ b/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); +} \ No newline at end of file diff --git a/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.qhelp b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.qhelp new file mode 100644 index 000000000000..d09a7cf923c4 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.qhelp @@ -0,0 +1,22 @@ + + + + +

If a ResultReceiver is obtained from an untrusted source, such as an Intent received by an exported component, + do not send it sensitive data. Otherwise, the information may be leaked to a malicious application.

+
+ + +

+ Do not send sensitive data to an untrusted ResultReceiver. +

+
+ + +

In the following (bad) example, sensitive data is sent to an untrusted ResultReceiver.

+ +
+ + + +
diff --git a/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql new file mode 100644 index 000000000000..efbb0e4c11ec --- /dev/null +++ b/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" diff --git a/java/ql/src/change-notes/2023-01-12-sensitive-result-receiver.md b/java/ql/src/change-notes/2023-01-12-sensitive-result-receiver.md new file mode 100644 index 000000000000..12ed2449246b --- /dev/null +++ b/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`. \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.expected b/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.java b/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.java new file mode 100644 index 000000000000..eb673b635c1c --- /dev/null +++ b/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 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 + } +} \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.ql b/java/ql/test/query-tests/security/CWE-927/SensitiveResultReceiver.ql new file mode 100644 index 000000000000..e8f2b4eef75c --- /dev/null +++ b/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 = "" + ) + } +}