From cd661f1d9fdf8dac5322b12d2cf58fc3cbc8eda8 Mon Sep 17 00:00:00 2001 From: Ed Minnix Date: Mon, 3 Apr 2023 14:01:54 -0400 Subject: [PATCH] Refactor SensitiveResultReceiver --- .../security/SensitiveResultReceiverQuery.qll | 39 +++++++++---------- .../CWE/CWE-927/SensitiveResultReceiver.ql | 6 ++- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll index 0679f66b575..ae1042b4a88 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll @@ -17,45 +17,44 @@ private class ResultReceiverSendCall extends MethodAccess { Expr getSentData() { result = this.getArgument(1) } } -private class UntrustedResultReceiverConf extends TaintTracking2::Configuration { - UntrustedResultReceiverConf() { this = "UntrustedResultReceiverConf" } +private module UntrustedResultReceiverConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource } - override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node node) { + predicate isSink(DataFlow::Node node) { node.asExpr() = any(ResultReceiverSendCall c).getReceiver() } } +private module UntrustedResultReceiverFlow = TaintTracking::Global; + private predicate untrustedResultReceiverSend(DataFlow::Node src, ResultReceiverSendCall call) { - any(UntrustedResultReceiverConf c).hasFlow(src, DataFlow::exprNode(call.getReceiver())) + UntrustedResultReceiverFlow::flow(src, DataFlow::exprNode(call.getReceiver())) } -private class SensitiveResultReceiverConf extends TaintTracking::Configuration { - SensitiveResultReceiverConf() { this = "SensitiveResultReceiverConf" } +private module SensitiveResultReceiverConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr } - override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr } - - override predicate isSink(DataFlow::Node node) { + 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) - } + predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { 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`. */ +module SensitiveResultReceiverFlow = TaintTracking::Global; + +/** + * 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 + SensitiveResultReceiverFlow::PathNode src, SensitiveResultReceiverFlow::PathNode sink, + DataFlow::Node recSrc ) { - exists(ResultReceiverSendCall call, SensitiveResultReceiverConf conf | - conf.hasFlowPath(src, sink) and + exists(ResultReceiverSendCall call | + SensitiveResultReceiverFlow::flowPath(src, sink) and sink.getNode().asExpr() = call.getSentData() and untrustedResultReceiverSend(recSrc, call) ) diff --git a/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql index efbb0e4c11e..367706634c6 100644 --- a/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql +++ b/java/ql/src/Security/CWE/CWE-927/SensitiveResultReceiver.ql @@ -13,9 +13,11 @@ import java import semmle.code.java.security.SensitiveResultReceiverQuery -import DataFlow::PathGraph +import SensitiveResultReceiverFlow::PathGraph -from DataFlow::PathNode src, DataFlow::PathNode sink, DataFlow::Node recSrc +from + SensitiveResultReceiverFlow::PathNode src, SensitiveResultReceiverFlow::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"