From 8f940fce1a8b9044be47125dd3a68f3acd85f5d1 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 23 Apr 2024 13:27:08 +0200 Subject: [PATCH] Java: Indentify more APIs as supported in the telemetry queries (as QL defined sinks). --- .../semmle/code/java/dataflow/ApiSinks.qll | 122 ++++++++++++++++++ .../AndroidSensitiveCommunicationQuery.qll | 17 ++- .../CleartextStorageAndroidDatabaseQuery.qll | 12 +- ...CleartextStorageAndroidFilesystemQuery.qll | 15 ++- .../security/CleartextStorageCookieQuery.qll | 9 +- .../CleartextStorageSharedPrefsQuery.qll | 15 ++- .../ExternallyControlledFormatStringQuery.qll | 11 +- .../security/SensitiveResultReceiverQuery.qll | 17 ++- .../code/java/security/SensitiveUiQuery.qll | 19 ++- ...TempDirLocalInformationDisclosureQuery.qll | 17 ++- .../security/WebviewDebuggingEnabledQuery.qll | 19 ++- java/ql/src/Telemetry/ExternalApi.qll | 3 +- 12 files changed, 232 insertions(+), 44 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll diff --git a/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll b/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll new file mode 100644 index 0000000000000..af4886e080ed3 --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll @@ -0,0 +1,122 @@ +/** Provides classes representing various flow sinks for data flow / taint tracking. */ + +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.ExternalFlow + +/** + * A data flow sink node. + */ +abstract class SinkNode extends DataFlow::Node { } + +/** + * Module that adds all API like sinks to `SinkNode`, excluding sinks for cryptography based + * queries, and queries where sinks are not succifiently defined (eg. using broad method name matching). + */ +private module ApiSources { + private import semmle.code.java.security.AndroidSensitiveCommunicationQuery as AndroidSensitiveCommunicationQuery + private import semmle.code.java.security.ArbitraryApkInstallation as ArbitraryApkInstallation + private import semmle.code.java.security.CleartextStorageAndroidDatabaseQuery as CleartextStorageAndroidDatabaseQuery + private import semmle.code.java.security.CleartextStorageAndroidFilesystemQuery as CleartextStorageAndroidFilesystemQuery + private import semmle.code.java.security.CleartextStorageCookieQuery as CleartextStorageCookieQuery + private import semmle.code.java.security.CleartextStorageSharedPrefsQuery as CleartextStorageSharedPrefsQuery + private import semmle.code.java.security.ExternallyControlledFormatStringQuery as ExternallyControlledFormatStringQuery + private import semmle.code.java.security.InsecureBasicAuth as InsecureBasicAuth + private import semmle.code.java.security.IntentUriPermissionManipulation as IntentUriPermissionManipulation + private import semmle.code.java.security.InsecureLdapAuth as InsecureLdapAuth + private import semmle.code.java.security.InsecureTrustManager as InsecureTrustManager + private import semmle.code.java.security.JndiInjection as JndiInjection + private import semmle.code.java.security.JWT as Jwt + private import semmle.code.java.security.OgnlInjection as OgnlInjection + private import semmle.code.java.security.SensitiveResultReceiverQuery as SensitiveResultReceiverQuery + private import semmle.code.java.security.SensitiveUiQuery as SensitiveUiQuery + private import semmle.code.java.security.SpelInjection as SpelInjection + private import semmle.code.java.security.SpelInjectionQuery as SpelInjectionQuery + private import semmle.code.java.security.QueryInjection as QueryInjection + private import semmle.code.java.security.TempDirLocalInformationDisclosureQuery as TempDirLocalInformationDisclosureQuery + private import semmle.code.java.security.UnsafeAndroidAccess as UnsafeAndroidAccess + private import semmle.code.java.security.UnsafeContentUriResolution as UnsafeContentUriResolution + private import semmle.code.java.security.UnsafeDeserializationQuery as UnsafeDeserializationQuery + private import semmle.code.java.security.UrlRedirect as UrlRedirect + private import semmle.code.java.security.WebviewDebuggingEnabledQuery as WebviewDebuggingEnabledQuery + private import semmle.code.java.security.XPath as Xpath + private import semmle.code.java.security.XSS as Xss + + private class AndoidIntentRedirectionQuerySinks extends SinkNode instanceof AndroidSensitiveCommunicationQuery::SensitiveCommunicationSink + { } + + private class ArbitraryApkInstallationSinks extends SinkNode instanceof ArbitraryApkInstallation::SetDataSink + { } + + private class CleartextStorageAndroidDatabaseQuerySinks extends SinkNode instanceof CleartextStorageAndroidDatabaseQuery::LocalDatabaseSink + { } + + private class CleartextStorageAndroidFilesystemQuerySinks extends SinkNode instanceof CleartextStorageAndroidFilesystemQuery::LocalFileSink + { } + + private class CleartextStorageCookieQuerySinks extends SinkNode instanceof CleartextStorageCookieQuery::CookieStoreSink + { } + + private class CleartextStorageSharedPrefsQuerySinks extends SinkNode instanceof CleartextStorageSharedPrefsQuery::SharedPreferencesSink + { } + + private class ExternallyControlledFormatStringQuerySinks extends SinkNode instanceof ExternallyControlledFormatStringQuery::StringFormatSink + { } + + private class InsecureBasicAuthSinks extends SinkNode instanceof InsecureBasicAuth::InsecureBasicAuthSink + { } + + private class InsecureTrustManagerSinks extends SinkNode instanceof InsecureTrustManager::InsecureTrustManagerSink + { } + + private class IntentUriPermissionManipulationSinks extends SinkNode instanceof IntentUriPermissionManipulation::IntentUriPermissionManipulationSink + { } + + private class InsecureLdapAuthSinks extends SinkNode instanceof InsecureLdapAuth::InsecureLdapUrlSink + { } + + private class JndiInjectionSinks extends SinkNode instanceof JndiInjection::JndiInjectionSink { } + + private class JwtSinks extends SinkNode instanceof Jwt::JwtParserWithInsecureParseSink { } + + private class OgnlInjectionSinks extends SinkNode instanceof OgnlInjection::OgnlInjectionSink { } + + private class SensitiveResultReceiverQuerySinks extends SinkNode instanceof SensitiveResultReceiverQuery::SensitiveResultReceiverSink + { } + + private class SensitiveUiQuerySinks extends SinkNode instanceof SensitiveUiQuery::TextFieldSink { + } + + private class SpelInjectionSinks extends SinkNode instanceof SpelInjection::SpelExpressionEvaluationSink + { } + + private class QueryInjectionSinks extends SinkNode instanceof QueryInjection::QueryInjectionSink { + } + + private class TempDirLocalInformationDisclosureSinks extends SinkNode instanceof TempDirLocalInformationDisclosureQuery::MethodFileDirectoryCreationSink + { } + + private class UnsafeAndroidAccessSinks extends SinkNode instanceof UnsafeAndroidAccess::UrlResourceSink + { } + + private class UnsafeContentUriResolutionSinks extends SinkNode instanceof UnsafeContentUriResolution::ContentUriResolutionSink + { } + + private class UnsafeDeserializationQuerySinks extends SinkNode instanceof UnsafeDeserializationQuery::UnsafeDeserializationSink + { } + + private class UrlRedirectSinks extends SinkNode instanceof UrlRedirect::UrlRedirectSink { } + + private class WebviewDebugEnabledQuery extends SinkNode instanceof WebviewDebuggingEnabledQuery::WebviewDebugSink + { } + + private class XPathSinks extends SinkNode instanceof Xpath::XPathInjectionSink { } + + private class XssSinks extends SinkNode instanceof Xss::XssSink { } + + /** + * Add all models as data sinks. + */ + private class SinkNodeExternal extends SinkNode { + SinkNodeExternal() { sinkNode(this, _) } + } +} diff --git a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll index 53adc7132c5e9..9773d00849fd1 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll @@ -151,17 +151,24 @@ deprecated class SensitiveCommunicationConfig extends TaintTracking::Configurati } } +/** + * A class of sensitive communication sink nodes. + */ +class SensitiveCommunicationSink extends DataFlow::Node { + SensitiveCommunicationSink() { + isSensitiveBroadcastSink(this) + or + isStartActivityOrServiceSink(this) + } +} + /** * Taint configuration tracking flow from variables containing sensitive information to broadcast Intents. */ module SensitiveCommunicationConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveInfoExpr } - predicate isSink(DataFlow::Node sink) { - isSensitiveBroadcastSink(sink) - or - isStartActivityOrServiceSink(sink) - } + predicate isSink(DataFlow::Node sink) { sink instanceof SensitiveCommunicationSink } /** * Holds if broadcast doesn't specify receiving package name of the 3rd party app diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index 7f306298a45eb..5d212ea45f23a 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -103,13 +103,17 @@ class LocalDatabaseOpenMethodCallSource extends DataFlow::Node { LocalDatabaseOpenMethodCallSource() { this.asExpr() instanceof LocalDatabaseOpenMethodCall } } +/** + * A class of local database sink nodes. + */ +class LocalDatabaseSink extends DataFlow::Node { + LocalDatabaseSink() { localDatabaseInput(this, _) or localDatabaseStore(this, _) } +} + private module LocalDatabaseFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof LocalDatabaseOpenMethodCallSource } - predicate isSink(DataFlow::Node sink) { - localDatabaseInput(sink, _) or - localDatabaseStore(sink, _) - } + predicate isSink(DataFlow::Node sink) { sink instanceof LocalDatabaseSink } predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { // Adds a step for tracking databases through field flow, that is, a database is opened and diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index 0759b4c061b08..90749120fce6b 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -86,13 +86,20 @@ class LocalFileOpenCallSource extends DataFlow::Node { LocalFileOpenCallSource() { this.asExpr() instanceof LocalFileOpenCall } } +/** + * A class of local file sink nodes. + */ +class LocalFileSink extends DataFlow::Node { + LocalFileSink() { + filesystemInput(this, _) or + closesFile(this, _) + } +} + private module FilesystemFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof LocalFileOpenCallSource } - predicate isSink(DataFlow::Node sink) { - filesystemInput(sink, _) or - closesFile(sink, _) - } + predicate isSink(DataFlow::Node sink) { sink instanceof LocalFileSink } predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // Add nested Writer constructors as extra data flow steps diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index af55c29e91c4e..379d52eb54972 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -44,10 +44,17 @@ class CookieSource extends DataFlow::Node { CookieSource() { this.asExpr() instanceof Cookie } } +/** + * A class of cookie store sink nodes. + */ +class CookieStoreSink extends DataFlow::Node { + CookieStoreSink() { cookieStore(this, _) } +} + private module CookieToStoreFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof CookieSource } - predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) } + predicate isSink(DataFlow::Node sink) { sink instanceof CookieStoreSink } } private module CookieToStoreFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index 3f690aeb6f19b..c09fb3cc61a65 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -76,14 +76,21 @@ class SharedPreferencesEditorMethodCallSource extends DataFlow::Node { } } +/** + * A class of shared preferences sink nodes. + */ +class SharedPreferencesSink extends DataFlow::Node { + SharedPreferencesSink() { + sharedPreferencesInput(this, _) or + sharedPreferencesStore(this, _) + } +} + /** Flow from `SharedPreferences.Editor` to either a setter or a store method. */ private module SharedPreferencesFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof SharedPreferencesEditorMethodCallSource } - predicate isSink(DataFlow::Node sink) { - sharedPreferencesInput(sink, _) or - sharedPreferencesStore(sink, _) - } + predicate isSink(DataFlow::Node sink) { sink instanceof SharedPreferencesSink } } private module SharedPreferencesFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll index a71ebc964f64e..2fc622325deaf 100644 --- a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll @@ -4,15 +4,20 @@ import java private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.StringFormat +/** + * A class of string format sink nodes. + */ +class StringFormatSink extends DataFlow::Node { + StringFormatSink() { this.asExpr() = any(StringFormat formatCall).getFormatArgument() } +} + /** * A taint-tracking configuration for externally controlled format string vulnerabilities. */ module ExternallyControlledFormatStringConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource } - predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(StringFormat formatCall).getFormatArgument() - } + predicate isSink(DataFlow::Node sink) { sink instanceof StringFormatSink } predicate isBarrier(DataFlow::Node node) { node.getType() instanceof NumericType or node.getType() instanceof BooleanType diff --git a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll index 63d6d88d83cb4..13a4b562a50e3 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll @@ -50,15 +50,22 @@ deprecated private class SensitiveResultReceiverConf extends TaintTracking::Conf } } -private module SensitiveResultReceiverConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr } - - predicate isSink(DataFlow::Node node) { +/** + * A class of sensitive result receiver sink nodes. + */ +class SensitiveResultReceiverSink extends DataFlow::Node { + SensitiveResultReceiverSink() { exists(ResultReceiverSendCall call | untrustedResultReceiverSend(_, call) and - node.asExpr() = call.getSentData() + this.asExpr() = call.getSentData() ) } +} + +private module SensitiveResultReceiverConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr } + + predicate isSink(DataFlow::Node node) { node instanceof SensitiveResultReceiverSink } predicate allowImplicitRead(DataFlow::Node n, DataFlow::ContentSet c) { isSink(n) and exists(c) } } diff --git a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll index 38a5aeb93cf96..884ab40a3239c 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll @@ -53,16 +53,23 @@ private class MaskCall extends MethodCall { } } -/** A configuration for tracking sensitive information to text fields. */ -private module TextFieldTrackingConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } - - predicate isSink(DataFlow::Node sink) { +/** + * A class of test field sink nodes. + */ +class TextFieldSink extends DataFlow::Node { + TextFieldSink() { exists(SetTextCall call | - sink.asExpr() = call.getStringArgument() and + this.asExpr() = call.getStringArgument() and not setTextCallIsMasked(call) ) } +} + +/** A configuration for tracking sensitive information to text fields. */ +private module TextFieldTrackingConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } + + predicate isSink(DataFlow::Node sink) { sink instanceof TextFieldSink } predicate isBarrier(DataFlow::Node node) { node instanceof SimpleTypeSanitizer } diff --git a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll index 843db3b5934b6..970363fe54397 100644 --- a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll @@ -153,6 +153,17 @@ module TempDirSystemGetPropertyToCreateConfig implements DataFlow::ConfigSig { module TempDirSystemGetPropertyToCreate = TaintTracking::Global; +/** + * A class of method file directory creation sink nodes. + */ +class MethodFileDirectoryCreationSink extends DataFlow::Node { + MethodFileDirectoryCreationSink() { + exists(MethodCall ma | ma.getMethod() instanceof MethodFileDirectoryCreation | + ma.getQualifier() = this.asExpr() + ) + } +} + /** * Configuration that tracks calls to to `mkdir` or `mkdirs` that are are directly on the temp directory system property. * Examples: @@ -172,11 +183,7 @@ module TempDirSystemGetPropertyDirectlyToMkdirConfig implements DataFlow::Config ) } - predicate isSink(DataFlow::Node node) { - exists(MethodCall ma | ma.getMethod() instanceof MethodFileDirectoryCreation | - ma.getQualifier() = node.asExpr() - ) - } + predicate isSink(DataFlow::Node node) { node instanceof MethodFileDirectoryCreationSink } predicate isBarrier(DataFlow::Node sanitizer) { isFileConstructorArgument(sanitizer.asExpr(), _, _) diff --git a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll index 00b9c715f7522..d2a21be95e0a6 100644 --- a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll +++ b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll @@ -44,18 +44,25 @@ deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration { } } +/** + * A class of webview debug sink nodes. + */ +class WebviewDebugSink extends DataFlow::Node { + WebviewDebugSink() { + exists(MethodCall ma | + ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and + this.asExpr() = ma.getArgument(0) + ) + } +} + /** A configuration to find instances of `setWebContentDebuggingEnabled` called with `true` values. */ module WebviewDebugEnabledConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { node.asExpr().(BooleanLiteral).getBooleanValue() = true } - predicate isSink(DataFlow::Node node) { - exists(MethodCall ma | - ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and - node.asExpr() = ma.getArgument(0) - ) - } + predicate isSink(DataFlow::Node node) { node instanceof WebviewDebugSink } predicate isBarrier(DataFlow::Node node) { exists(Guard debug | isDebugCheck(debug) and debug.controls(node.asExpr().getBasicBlock(), _)) diff --git a/java/ql/src/Telemetry/ExternalApi.qll b/java/ql/src/Telemetry/ExternalApi.qll index cbf2875e9d1fc..a548593c36a3b 100644 --- a/java/ql/src/Telemetry/ExternalApi.qll +++ b/java/ql/src/Telemetry/ExternalApi.qll @@ -2,6 +2,7 @@ private import java private import semmle.code.java.dataflow.ApiSources as ApiSources +private import semmle.code.java.dataflow.ApiSinks as ApiSinks private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources @@ -74,7 +75,7 @@ class ExternalApi extends Callable { /** Holds if this API is a known sink. */ pragma[nomagic] - predicate isSink() { sinkNode(this.getAnInput(), _) } + predicate isSink() { this.getAnInput() instanceof ApiSinks::SinkNode } /** Holds if this API is a known neutral. */ pragma[nomagic]