From 06f987ad580466386d5a076eb8602feda214b357 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 23 Apr 2024 15:43:25 +0200 Subject: [PATCH 1/7] Java: Add test example of a supported sink defined in QL. --- .../SupportedExternalApis.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.java b/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.java index 880d183aecd..8fd5f679824 100644 --- a/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.java +++ b/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.java @@ -1,10 +1,13 @@ +import java.io.File; +import java.io.FileWriter; +import java.io.InputStream; +import java.io.IOException; +import java.net.URL; import java.time.Duration; import java.util.HashMap; import java.util.Map; -import java.io.InputStream; -import java.net.URL; -import java.io.File; -import java.io.FileWriter; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.ServletException; import org.apache.commons.io.FileUtils; class SupportedExternalApis { @@ -30,4 +33,8 @@ class SupportedExternalApis { file.compareTo(file); // supported neutral sink (compareTo) } + + public static void doSendRedirect(HttpServletResponse req) throws ServletException, IOException { + req.sendRedirect("myredirect"); // supported sink (sendRedirect) + } } From acb2bbb2a36702d9c0d7b08c3ddab6103e1a12ff Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 22 Apr 2024 11:21:13 +0200 Subject: [PATCH 2/7] Java: Identify more APIs as supported in the telemetry queries (as QL defined sources). --- .../semmle/code/java/dataflow/ApiSources.qll | 69 +++++++++++++++++++ .../semmle/code/java/dataflow/FlowSources.qll | 8 ++- .../CleartextStorageAndroidDatabaseQuery.qll | 11 ++- ...CleartextStorageAndroidFilesystemQuery.qll | 9 ++- .../security/CleartextStorageCookieQuery.qll | 9 ++- .../CleartextStorageSharedPrefsQuery.qll | 13 +++- .../ImproperIntentVerificationQuery.qll | 13 +++- .../java/security/StackTraceExposureQuery.qll | 7 +- java/ql/lib/semmle/code/java/security/XSS.qll | 9 ++- .../code/java/security/ZipSlipQuery.qll | 13 +++- java/ql/src/Telemetry/ExternalApi.qll | 5 +- 11 files changed, 143 insertions(+), 23 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/dataflow/ApiSources.qll diff --git a/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll b/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll new file mode 100644 index 00000000000..61025262cb5 --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll @@ -0,0 +1,69 @@ +/** Provides classes representing various flow sources for data flow / taint tracking. */ + +private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.ExternalFlow + +/** + * A data flow source node. + */ +abstract class SourceNode extends DataFlow::Node { } + +/** + * Module that adds all API like sources to `SourceNode`, excluding some sources for cryptography based + * queries, and queries where sources are not succifiently defined (eg. using broad method name matching). + */ +private module ApiSources { + private import FlowSources as FlowSources + 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.ImplicitPendingIntentsQuery as ImplicitPendingIntentsQuery + private import semmle.code.java.security.ImproperIntentVerificationQuery as ImproperIntentVerificationQuery + private import semmle.code.java.security.InsecureTrustManager as InsecureTrustManager + private import semmle.code.java.security.JWT as Jwt + private import semmle.code.java.security.StackTraceExposureQuery as StackTraceExposureQuery + private import semmle.code.java.security.ZipSlipQuery as ZipSlipQuery + + private class FlowSourcesSourceNode extends SourceNode instanceof FlowSources::SourceNode { } + + private class ArbitraryApkInstallationSources extends SourceNode instanceof ArbitraryApkInstallation::ExternalApkSource + { } + + private class CleartextStorageAndroidDatabaseQuerySources extends SourceNode instanceof CleartextStorageAndroidDatabaseQuery::LocalDatabaseOpenMethodCallSource + { } + + private class CleartextStorageAndroidFilesystemQuerySources extends SourceNode instanceof CleartextStorageAndroidFilesystemQuery::LocalFileOpenCallSource + { } + + private class CleartextStorageCookieQuerySources extends SourceNode instanceof CleartextStorageCookieQuery::CookieSource + { } + + private class CleartextStorageSharedPrefsQuerySources extends SourceNode instanceof CleartextStorageSharedPrefsQuery::SharedPreferencesEditorMethodCallSource + { } + + private class ImplicitPendingIntentsQuerySources extends SourceNode instanceof ImplicitPendingIntentsQuery::ImplicitPendingIntentSource + { } + + private class ImproperIntentVerificationQuerySources extends SourceNode instanceof ImproperIntentVerificationQuery::VerifiedIntentConfigSource + { } + + private class InsecureTrustManagerSources extends SourceNode instanceof InsecureTrustManager::InsecureTrustManagerSource + { } + + private class JwtSources extends SourceNode instanceof Jwt::JwtParserWithInsecureParseSource { } + + private class StackTraceExposureQuerySources extends SourceNode instanceof StackTraceExposureQuery::GetMessageFlowSource + { } + + private class ZipSlipQuerySources extends SourceNode instanceof ZipSlipQuery::ArchiveEntryNameMethodSource + { } + + /** + * Add all models as data sources. + */ + private class SourceNodeExternal extends SourceNode { + SourceNodeExternal() { sourceNode(this, _) } + } +} diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll index 425eb3ccaa6..49d7bda4e44 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll @@ -194,15 +194,17 @@ private class AndroidExternalStorageSource extends RemoteFlowSource { } /** Class for `tainted` user input. */ -abstract class UserInput extends DataFlow::Node { } +abstract class UserInput extends SourceNode { } /** * Input that may be controlled by a remote user. */ -private class RemoteUserInput extends UserInput instanceof RemoteFlowSource { } +private class RemoteUserInput extends UserInput instanceof RemoteFlowSource { + override string getThreatModel() { result = RemoteFlowSource.super.getThreatModel() } +} /** A node with input that may be controlled by a local user. */ -abstract class LocalUserInput extends UserInput, SourceNode { +abstract class LocalUserInput extends UserInput { override string getThreatModel() { result = "local" } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index b42389a1df6..7f306298a45 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -96,10 +96,15 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodCall store) ) } +/** + * A class of local database open method call source nodes. + */ +class LocalDatabaseOpenMethodCallSource extends DataFlow::Node { + LocalDatabaseOpenMethodCallSource() { this.asExpr() instanceof LocalDatabaseOpenMethodCall } +} + private module LocalDatabaseFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof LocalDatabaseOpenMethodCall - } + predicate isSource(DataFlow::Node source) { source instanceof LocalDatabaseOpenMethodCallSource } predicate isSink(DataFlow::Node sink) { localDatabaseInput(sink, _) or diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index d7097f1ecf2..0759b4c061b 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -79,8 +79,15 @@ private class CloseFileMethod extends Method { } } +/** + * A class of local file open call source nodes. + */ +class LocalFileOpenCallSource extends DataFlow::Node { + LocalFileOpenCallSource() { this.asExpr() instanceof LocalFileOpenCall } +} + private module FilesystemFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall } + predicate isSource(DataFlow::Node src) { src instanceof LocalFileOpenCallSource } predicate isSink(DataFlow::Node sink) { filesystemInput(sink, _) or diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index 2b6534e8cbd..af55c29e91c 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -37,8 +37,15 @@ private predicate cookieStore(DataFlow::Node cookie, Expr store) { ) } +/** + * A class of cookie source nodes. + */ +class CookieSource extends DataFlow::Node { + CookieSource() { this.asExpr() instanceof Cookie } +} + private module CookieToStoreFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie } + predicate isSource(DataFlow::Node src) { src instanceof CookieSource } predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index 39e1ffa3c75..3f690aeb6f1 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -67,11 +67,18 @@ private predicate sharedPreferencesStore(DataFlow::Node editor, MethodCall m) { editor.asExpr() = m.getQualifier().getUnderlyingExpr() } +/** + * A shared preferences editor method call source nodes. + */ +class SharedPreferencesEditorMethodCallSource extends DataFlow::Node { + SharedPreferencesEditorMethodCallSource() { + this.asExpr() instanceof SharedPreferencesEditorMethodCall + } +} + /** Flow from `SharedPreferences.Editor` to either a setter or a store method. */ private module SharedPreferencesFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof SharedPreferencesEditorMethodCall - } + predicate isSource(DataFlow::Node src) { src instanceof SharedPreferencesEditorMethodCallSource } predicate isSink(DataFlow::Node sink) { sharedPreferencesInput(sink, _) or diff --git a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll index bca045bc8e4..92bcac5b50e 100644 --- a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll @@ -13,11 +13,18 @@ private class OnReceiveMethod extends Method { Parameter getIntentParameter() { result = this.getParameter(1) } } +/** + * A class of verified intent source nodes. + */ +class VerifiedIntentConfigSource extends DataFlow::Node { + VerifiedIntentConfigSource() { + this.asParameter() = any(OnReceiveMethod orm).getIntentParameter() + } +} + /** A configuration to detect whether the `action` of an `Intent` is checked. */ private module VerifiedIntentConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { - src.asParameter() = any(OnReceiveMethod orm).getIntentParameter() - } + predicate isSource(DataFlow::Node src) { src instanceof VerifiedIntentConfigSource } predicate isSink(DataFlow::Node sink) { exists(MethodCall ma | diff --git a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll index dba9492d137..2e4b31b7785 100644 --- a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll @@ -19,7 +19,7 @@ private class PrintStackTraceMethod extends Method { } private module ServletWriterSourceToPrintStackTraceMethodFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource } + predicate isSource(DataFlow::Node src) { src instanceof XssVulnerableWriterSourceNode } predicate isSink(DataFlow::Node sink) { exists(MethodCall ma | @@ -95,7 +95,10 @@ predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stac ) } -private class GetMessageFlowSource extends DataFlow::Node { +/** + * A class of get message source nodes. + */ +class GetMessageFlowSource extends DataFlow::Node { GetMessageFlowSource() { exists(Method method | this.asExpr().(MethodCall).getMethod() = method | method.hasName("getMessage") and diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index 9edee5823bf..aa69e5e7865 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -62,7 +62,7 @@ private class DefaultXssSanitizer extends XssSanitizer { /** A configuration that tracks data from a servlet writer to an output method. */ private module XssVulnerableWriterSourceToWritingMethodFlowConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src.asExpr() instanceof XssVulnerableWriterSource } + predicate isSource(DataFlow::Node src) { src instanceof XssVulnerableWriterSourceNode } predicate isSink(DataFlow::Node sink) { exists(MethodCall ma | @@ -105,6 +105,13 @@ class XssVulnerableWriterSource extends MethodCall { } } +/** + * A class of xss vulnerable writer source nodes. + */ +class XssVulnerableWriterSourceNode extends DataFlow::Node { + XssVulnerableWriterSourceNode() { this.asExpr() instanceof XssVulnerableWriterSource } +} + /** * Holds if `s` is an HTTP Content-Type vulnerable to XSS. */ diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index 7ba99a31e26..f6f3b1bf27c 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -21,13 +21,20 @@ private class ArchiveEntryNameMethod extends Method { } } +/** + * A class of entry name method source nodes. + */ +class ArchiveEntryNameMethodSource extends DataFlow::Node { + ArchiveEntryNameMethodSource() { + this.asExpr().(MethodCall).getMethod() instanceof ArchiveEntryNameMethod + } +} + /** * A taint-tracking configuration for reasoning about unsafe zip file extraction. */ module ZipSlipConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node source) { - source.asExpr().(MethodCall).getMethod() instanceof ArchiveEntryNameMethod - } + predicate isSource(DataFlow::Node source) { source instanceof ArchiveEntryNameMethodSource } predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink } diff --git a/java/ql/src/Telemetry/ExternalApi.qll b/java/ql/src/Telemetry/ExternalApi.qll index 47527dfbb46..cbf2875e9d1 100644 --- a/java/ql/src/Telemetry/ExternalApi.qll +++ b/java/ql/src/Telemetry/ExternalApi.qll @@ -1,6 +1,7 @@ /** Provides classes and predicates related to handling APIs from external libraries. */ private import java +private import semmle.code.java.dataflow.ApiSources as ApiSources private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources @@ -69,9 +70,7 @@ class ExternalApi extends Callable { } pragma[nomagic] - predicate isSource() { - this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _) - } + predicate isSource() { this.getAnOutput() instanceof ApiSources::SourceNode } /** Holds if this API is a known sink. */ pragma[nomagic] From 9db32f4d26ac81ec32378a7d0efc2c55a8b1da15 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 23 Apr 2024 13:27:08 +0200 Subject: [PATCH 3/7] Java: Identify 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 | 19 ++- .../code/java/security/SensitiveUiQuery.qll | 19 ++- ...TempDirLocalInformationDisclosureQuery.qll | 17 ++- .../security/WebviewDebuggingEnabledQuery.qll | 19 ++- java/ql/src/Telemetry/ExternalApi.qll | 3 +- 12 files changed, 233 insertions(+), 45 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 00000000000..0dae848c15d --- /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 ApiSinks { + 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 53adc7132c5..9773d00849f 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 7f306298a45..5d212ea45f2 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 0759b4c061b..90749120fce 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 af55c29e91c..379d52eb549 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 3f690aeb6f1..c09fb3cc61a 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 a71ebc964f6..2fc622325de 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 63d6d88d83c..13a4b562a50 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 } } +/** + * A class of sensitive result receiver sink nodes. + */ +class SensitiveResultReceiverSink extends DataFlow::Node { + SensitiveResultReceiverSink() { + exists(ResultReceiverSendCall call | + untrustedResultReceiverSend(_, call) and + this.asExpr() = call.getSentData() + ) + } +} + private module SensitiveResultReceiverConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { node.asExpr() instanceof SensitiveExpr } - predicate isSink(DataFlow::Node node) { - exists(ResultReceiverSendCall call | - untrustedResultReceiverSend(_, call) and - node.asExpr() = call.getSentData() - ) - } + 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 38a5aeb93cf..884ab40a323 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 class of test field sink nodes. + */ +class TextFieldSink extends DataFlow::Node { + TextFieldSink() { + exists(SetTextCall call | + 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) { - exists(SetTextCall call | - sink.asExpr() = call.getStringArgument() and - not setTextCallIsMasked(call) - ) - } + 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 843db3b5934..970363fe543 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 00b9c715f75..d2a21be95e0 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 cbf2875e9d1..a548593c36a 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] From b754706e444ab66aa958c1c36206ae793ed4280a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Tue, 23 Apr 2024 15:45:16 +0200 Subject: [PATCH 4/7] Java: Update SupportedExternalApi expected test output. --- .../SupportedExternalApis/SupportedExternalApis.expected | 1 + 1 file changed, 1 insertion(+) diff --git a/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.expected b/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.expected index 68dc7169533..d08da075f3f 100644 --- a/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.expected +++ b/java/ql/test/query-tests/Telemetry/SupportedExternalApis/SupportedExternalApis.expected @@ -13,4 +13,5 @@ | java.util.Map#put(Object,Object) | 1 | | java.util.Map$Entry#getKey() | 1 | | java.util.Set#iterator() | 1 | +| javax.servlet.http.HttpServletResponse#sendRedirect(String) | 1 | | org.apache.commons.io.FileUtils#deleteDirectory(File) | 1 | From f95b33049e2dec145af2c47e47cbe08c0bcf3a94 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 25 Apr 2024 16:44:55 +0200 Subject: [PATCH 5/7] Java: Improve the Api sources and sinks implementation. --- .../semmle/code/java/dataflow/ApiSinks.qll | 143 ++++-------------- .../semmle/code/java/dataflow/ApiSources.qll | 74 ++------- .../semmle/code/java/dataflow/FlowSinks.qll | 18 +++ .../semmle/code/java/dataflow/FlowSources.qll | 15 ++ .../AndroidSensitiveCommunicationQuery.qll | 3 +- .../security/ArbitraryApkInstallation.qll | 5 +- .../CleartextStorageAndroidDatabaseQuery.qll | 6 +- ...CleartextStorageAndroidFilesystemQuery.qll | 8 +- .../security/CleartextStorageCookieQuery.qll | 6 +- .../CleartextStorageSharedPrefsQuery.qll | 6 +- .../ExternallyControlledFormatStringQuery.qll | 3 +- .../java/security/ImplicitPendingIntents.qll | 3 +- .../ImproperIntentVerificationQuery.qll | 3 +- .../code/java/security/InsecureBasicAuth.qll | 3 +- .../code/java/security/InsecureLdapAuth.qll | 3 +- .../java/security/InsecureTrustManager.qll | 5 +- .../IntentUriPermissionManipulation.qll | 3 +- java/ql/lib/semmle/code/java/security/JWT.qll | 6 +- .../code/java/security/JndiInjection.qll | 3 +- .../code/java/security/OgnlInjection.qll | 3 +- .../code/java/security/QueryInjection.qll | 3 +- .../security/SensitiveResultReceiverQuery.qll | 3 +- .../code/java/security/SensitiveUiQuery.qll | 5 +- .../code/java/security/SpelInjection.qll | 3 +- .../java/security/StackTraceExposureQuery.qll | 3 +- ...TempDirLocalInformationDisclosureQuery.qll | 3 +- .../java/security/UnsafeAndroidAccess.qll | 3 +- .../security/UnsafeContentUriResolution.qll | 3 +- .../security/UnsafeDeserializationQuery.qll | 3 +- .../semmle/code/java/security/UrlRedirect.qll | 5 +- .../security/WebviewDebuggingEnabledQuery.qll | 3 +- .../lib/semmle/code/java/security/XPath.qll | 3 +- java/ql/lib/semmle/code/java/security/XSS.qll | 6 +- .../code/java/security/ZipSlipQuery.qll | 3 +- 34 files changed, 154 insertions(+), 214 deletions(-) create mode 100644 java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll diff --git a/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll b/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll index 0dae848c15d..c600bb1672d 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll @@ -1,122 +1,39 @@ /** 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 +private import semmle.code.java.dataflow.FlowSinks as FlowSinks -/** - * A data flow sink node. - */ -abstract class SinkNode extends DataFlow::Node { } +final class SinkNode = FlowSinks::ApiSinkNode; /** * 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 ApiSinks { - 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, _) } - } +private module AllApiSinks { + private import semmle.code.java.security.AndroidSensitiveCommunicationQuery + private import semmle.code.java.security.ArbitraryApkInstallation + private import semmle.code.java.security.CleartextStorageAndroidDatabaseQuery + private import semmle.code.java.security.CleartextStorageAndroidFilesystemQuery + private import semmle.code.java.security.CleartextStorageCookieQuery + private import semmle.code.java.security.CleartextStorageSharedPrefsQuery + private import semmle.code.java.security.ExternallyControlledFormatStringQuery + private import semmle.code.java.security.InsecureBasicAuth + private import semmle.code.java.security.IntentUriPermissionManipulation + private import semmle.code.java.security.InsecureLdapAuth + private import semmle.code.java.security.InsecureTrustManager + private import semmle.code.java.security.JndiInjection + private import semmle.code.java.security.JWT + private import semmle.code.java.security.OgnlInjection + private import semmle.code.java.security.SensitiveResultReceiverQuery + private import semmle.code.java.security.SensitiveUiQuery + private import semmle.code.java.security.SpelInjection + private import semmle.code.java.security.SpelInjectionQuery + private import semmle.code.java.security.QueryInjection + private import semmle.code.java.security.TempDirLocalInformationDisclosureQuery + private import semmle.code.java.security.UnsafeAndroidAccess + private import semmle.code.java.security.UnsafeContentUriResolution + private import semmle.code.java.security.UnsafeDeserializationQuery + private import semmle.code.java.security.UrlRedirect + private import semmle.code.java.security.WebviewDebuggingEnabledQuery + private import semmle.code.java.security.XPath + private import semmle.code.java.security.XSS } diff --git a/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll b/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll index 61025262cb5..5f825ad5445 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll @@ -1,69 +1,23 @@ /** Provides classes representing various flow sources for data flow / taint tracking. */ -private import semmle.code.java.dataflow.DataFlow -private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources as FlowSources -/** - * A data flow source node. - */ -abstract class SourceNode extends DataFlow::Node { } +final class SourceNode = FlowSources::ApiSourceNode; /** * Module that adds all API like sources to `SourceNode`, excluding some sources for cryptography based * queries, and queries where sources are not succifiently defined (eg. using broad method name matching). */ -private module ApiSources { - private import FlowSources as FlowSources - 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.ImplicitPendingIntentsQuery as ImplicitPendingIntentsQuery - private import semmle.code.java.security.ImproperIntentVerificationQuery as ImproperIntentVerificationQuery - private import semmle.code.java.security.InsecureTrustManager as InsecureTrustManager - private import semmle.code.java.security.JWT as Jwt - private import semmle.code.java.security.StackTraceExposureQuery as StackTraceExposureQuery - private import semmle.code.java.security.ZipSlipQuery as ZipSlipQuery - - private class FlowSourcesSourceNode extends SourceNode instanceof FlowSources::SourceNode { } - - private class ArbitraryApkInstallationSources extends SourceNode instanceof ArbitraryApkInstallation::ExternalApkSource - { } - - private class CleartextStorageAndroidDatabaseQuerySources extends SourceNode instanceof CleartextStorageAndroidDatabaseQuery::LocalDatabaseOpenMethodCallSource - { } - - private class CleartextStorageAndroidFilesystemQuerySources extends SourceNode instanceof CleartextStorageAndroidFilesystemQuery::LocalFileOpenCallSource - { } - - private class CleartextStorageCookieQuerySources extends SourceNode instanceof CleartextStorageCookieQuery::CookieSource - { } - - private class CleartextStorageSharedPrefsQuerySources extends SourceNode instanceof CleartextStorageSharedPrefsQuery::SharedPreferencesEditorMethodCallSource - { } - - private class ImplicitPendingIntentsQuerySources extends SourceNode instanceof ImplicitPendingIntentsQuery::ImplicitPendingIntentSource - { } - - private class ImproperIntentVerificationQuerySources extends SourceNode instanceof ImproperIntentVerificationQuery::VerifiedIntentConfigSource - { } - - private class InsecureTrustManagerSources extends SourceNode instanceof InsecureTrustManager::InsecureTrustManagerSource - { } - - private class JwtSources extends SourceNode instanceof Jwt::JwtParserWithInsecureParseSource { } - - private class StackTraceExposureQuerySources extends SourceNode instanceof StackTraceExposureQuery::GetMessageFlowSource - { } - - private class ZipSlipQuerySources extends SourceNode instanceof ZipSlipQuery::ArchiveEntryNameMethodSource - { } - - /** - * Add all models as data sources. - */ - private class SourceNodeExternal extends SourceNode { - SourceNodeExternal() { sourceNode(this, _) } - } +private module AllApiSources { + private import semmle.code.java.security.ArbitraryApkInstallation + private import semmle.code.java.security.CleartextStorageAndroidDatabaseQuery + private import semmle.code.java.security.CleartextStorageAndroidFilesystemQuery + private import semmle.code.java.security.CleartextStorageCookieQuery + private import semmle.code.java.security.CleartextStorageSharedPrefsQuery + private import semmle.code.java.security.ImplicitPendingIntentsQuery + private import semmle.code.java.security.ImproperIntentVerificationQuery + private import semmle.code.java.security.InsecureTrustManager + private import semmle.code.java.security.JWT + private import semmle.code.java.security.StackTraceExposureQuery + private import semmle.code.java.security.ZipSlipQuery } diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll new file mode 100644 index 00000000000..3b7fd191779 --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll @@ -0,0 +1,18 @@ +/** Provides classes representing various flow sinks for data flow / taint tracking. */ + +private import java +private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.DataFlow + +/** + * A data flow sink node for an API, which should be considered + * supported for a modeling perspective. + */ +abstract class ApiSinkNode extends DataFlow::Node { } + +/** + * Add all models as data sinks. + */ +private class ApiSinkNodeExternal extends ApiSinkNode { + ApiSinkNodeExternal() { sinkNode(this, _) } +} diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll index 49d7bda4e44..f28cc998487 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll @@ -387,3 +387,18 @@ class AndroidJavascriptInterfaceMethodParameter extends RemoteFlowSource { result = "Parameter of method with JavascriptInterface annotation" } } + +/** + * A data flow source node for an API, which should be considered + * supported for a modeling perspective. + */ +abstract class ApiSourceNode extends DataFlow::Node { } + +private class AddSourceNodes extends ApiSourceNode instanceof SourceNode { } + +/** + * Add all models as data sources. + */ +private class ApiSourceNodeExternal extends ApiSourceNode { + ApiSourceNodeExternal() { sourceNode(this, _) } +} diff --git a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll index 9773d00849f..607ced09b2c 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.frameworks.android.Intent import semmle.code.java.security.SensitiveActions +private import semmle.code.java.dataflow.FlowSinks /** * Gets regular expression for matching names of Android variables that indicate the value being held contains sensitive information. @@ -154,7 +155,7 @@ deprecated class SensitiveCommunicationConfig extends TaintTracking::Configurati /** * A class of sensitive communication sink nodes. */ -class SensitiveCommunicationSink extends DataFlow::Node { +class SensitiveCommunicationSink extends ApiSinkNode { SensitiveCommunicationSink() { isSensitiveBroadcastSink(this) or diff --git a/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallation.qll b/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallation.qll index 3aa59286fcd..d7c5fe94f28 100644 --- a/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallation.qll +++ b/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallation.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.frameworks.android.Intent import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.FlowSources /** A string literal that represents the MIME type for Android APKs. */ @@ -48,7 +49,7 @@ class SetDataMethod extends Method { } /** A dataflow sink for the URI of an intent. */ -class SetDataSink extends DataFlow::ExprNode { +class SetDataSink extends ApiSinkNode, DataFlow::ExprNode { SetDataSink() { exists(MethodCall ma | this.getExpr() = ma.getQualifier() and @@ -69,7 +70,7 @@ class UriConstructorMethod extends Method { * A dataflow source representing the URIs which an APK not controlled by the * application may come from. Including external storage and web URLs. */ -class ExternalApkSource extends DataFlow::Node { +class ExternalApkSource extends ApiSourceNode { ExternalApkSource() { sourceNode(this, "android-external-storage-dir") or this.asExpr().(MethodCall).getMethod() instanceof UriConstructorMethod or diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index 5d212ea45f2..b4162f2c695 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -6,6 +6,8 @@ import semmle.code.java.frameworks.android.ContentProviders import semmle.code.java.frameworks.android.Intent import semmle.code.java.frameworks.android.SQLite import semmle.code.java.security.CleartextStorageQuery +private import semmle.code.java.dataflow.FlowSinks +private import semmle.code.java.dataflow.FlowSources private class LocalDatabaseCleartextStorageSink extends CleartextStorageSink { LocalDatabaseCleartextStorageSink() { localDatabaseInput(_, this.asExpr()) } @@ -99,14 +101,14 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodCall store) /** * A class of local database open method call source nodes. */ -class LocalDatabaseOpenMethodCallSource extends DataFlow::Node { +class LocalDatabaseOpenMethodCallSource extends ApiSourceNode { LocalDatabaseOpenMethodCallSource() { this.asExpr() instanceof LocalDatabaseOpenMethodCall } } /** * A class of local database sink nodes. */ -class LocalDatabaseSink extends DataFlow::Node { +class LocalDatabaseSink extends ApiSinkNode { LocalDatabaseSink() { localDatabaseInput(this, _) or localDatabaseStore(this, _) } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index 90749120fce..8b1af7b4971 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -5,9 +5,11 @@ import java import semmle.code.java.dataflow.DataFlow -private import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.security.CleartextStorageQuery import semmle.code.xml.AndroidManifest +private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks +private import semmle.code.java.dataflow.FlowSources private class AndroidFilesystemCleartextStorageSink extends CleartextStorageSink { AndroidFilesystemCleartextStorageSink() { @@ -82,14 +84,14 @@ private class CloseFileMethod extends Method { /** * A class of local file open call source nodes. */ -class LocalFileOpenCallSource extends DataFlow::Node { +class LocalFileOpenCallSource extends ApiSourceNode { LocalFileOpenCallSource() { this.asExpr() instanceof LocalFileOpenCall } } /** * A class of local file sink nodes. */ -class LocalFileSink extends DataFlow::Node { +class LocalFileSink extends ApiSinkNode { LocalFileSink() { filesystemInput(this, _) or closesFile(this, _) diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index 379d52eb549..c3684646bdd 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -4,6 +4,8 @@ import java import semmle.code.java.dataflow.DataFlow deprecated import semmle.code.java.dataflow.DataFlow3 import semmle.code.java.security.CleartextStorageQuery +private import semmle.code.java.dataflow.FlowSinks +private import semmle.code.java.dataflow.FlowSources private class CookieCleartextStorageSink extends CleartextStorageSink { CookieCleartextStorageSink() { this.asExpr() = cookieInput(_) } @@ -40,14 +42,14 @@ private predicate cookieStore(DataFlow::Node cookie, Expr store) { /** * A class of cookie source nodes. */ -class CookieSource extends DataFlow::Node { +class CookieSource extends ApiSourceNode { CookieSource() { this.asExpr() instanceof Cookie } } /** * A class of cookie store sink nodes. */ -class CookieStoreSink extends DataFlow::Node { +class CookieStoreSink extends ApiSinkNode { CookieStoreSink() { cookieStore(this, _) } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index c09fb3cc61a..80dc2fca1f4 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -4,6 +4,8 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.frameworks.android.SharedPreferences import semmle.code.java.security.CleartextStorageQuery +private import semmle.code.java.dataflow.FlowSinks +private import semmle.code.java.dataflow.FlowSources private class SharedPrefsCleartextStorageSink extends CleartextStorageSink { SharedPrefsCleartextStorageSink() { @@ -70,7 +72,7 @@ private predicate sharedPreferencesStore(DataFlow::Node editor, MethodCall m) { /** * A shared preferences editor method call source nodes. */ -class SharedPreferencesEditorMethodCallSource extends DataFlow::Node { +class SharedPreferencesEditorMethodCallSource extends ApiSourceNode { SharedPreferencesEditorMethodCallSource() { this.asExpr() instanceof SharedPreferencesEditorMethodCall } @@ -79,7 +81,7 @@ class SharedPreferencesEditorMethodCallSource extends DataFlow::Node { /** * A class of shared preferences sink nodes. */ -class SharedPreferencesSink extends DataFlow::Node { +class SharedPreferencesSink extends ApiSinkNode { SharedPreferencesSink() { sharedPreferencesInput(this, _) or sharedPreferencesStore(this, _) diff --git a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll index 2fc622325de..8d6fe0426c3 100644 --- a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll @@ -1,13 +1,14 @@ /** Provides a taint-tracking configuration to reason about externally controlled format string vulnerabilities. */ import java +private import semmle.code.java.dataflow.FlowSinks 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 { +class StringFormatSink extends ApiSinkNode { StringFormatSink() { this.asExpr() = any(StringFormat formatCall).getFormatArgument() } } diff --git a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll index 4cac7715b98..5c4094de3d3 100644 --- a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll +++ b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.frameworks.android.Intent private import semmle.code.java.frameworks.android.PendingIntent @@ -27,7 +28,7 @@ class NoState extends PendingIntentState, TNoState { } /** A source for an implicit `PendingIntent` flow. */ -abstract class ImplicitPendingIntentSource extends DataFlow::Node { +abstract class ImplicitPendingIntentSource extends ApiSourceNode { /** * DEPRECATED: Open-ended flow state is not intended to be part of the extension points. * diff --git a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll index 92bcac5b50e..e8bfc97b0fc 100644 --- a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.xml.AndroidManifest import semmle.code.java.frameworks.android.Intent +private import semmle.code.java.dataflow.FlowSources /** An `onReceive` method of a `BroadcastReceiver` */ private class OnReceiveMethod extends Method { @@ -16,7 +17,7 @@ private class OnReceiveMethod extends Method { /** * A class of verified intent source nodes. */ -class VerifiedIntentConfigSource extends DataFlow::Node { +class VerifiedIntentConfigSource extends ApiSourceNode { VerifiedIntentConfigSource() { this.asParameter() = any(OnReceiveMethod orm).getIntentParameter() } diff --git a/java/ql/lib/semmle/code/java/security/InsecureBasicAuth.qll b/java/ql/lib/semmle/code/java/security/InsecureBasicAuth.qll index df9b6bdf4a1..b21492406ad 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureBasicAuth.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureBasicAuth.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.HttpsUrls +private import semmle.code.java.dataflow.FlowSinks /** * A source that represents HTTP URLs. @@ -20,7 +21,7 @@ private class DefaultInsecureBasicAuthSource extends InsecureBasicAuthSource { * A sink that represents a method that sets Basic Authentication. * Extend this class to add your own Insecure Basic Authentication sinks. */ -abstract class InsecureBasicAuthSink extends DataFlow::Node { } +abstract class InsecureBasicAuthSink extends ApiSinkNode { } /** A default sink representing methods that set an Authorization header. */ private class DefaultInsecureBasicAuthSink extends InsecureBasicAuthSink { diff --git a/java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll b/java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll index 9a8cd91b1fc..52d58afc9e7 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.frameworks.Networking private import semmle.code.java.frameworks.Jndi @@ -32,7 +33,7 @@ class InsecureLdapUrl extends Expr { /** * A sink representing the construction of a `DirContextEnvironment`. */ -class InsecureLdapUrlSink extends DataFlow::Node { +class InsecureLdapUrlSink extends ApiSinkNode { InsecureLdapUrlSink() { exists(ConstructorCall cc | cc.getConstructedType().getAnAncestor() instanceof TypeDirContext and diff --git a/java/ql/lib/semmle/code/java/security/InsecureTrustManager.qll b/java/ql/lib/semmle/code/java/security/InsecureTrustManager.qll index d82f088cf15..41d8f28573c 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureTrustManager.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureTrustManager.qll @@ -2,11 +2,12 @@ import java private import semmle.code.java.controlflow.Guards +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.security.Encryption private import semmle.code.java.security.SecurityFlag /** The creation of an insecure `TrustManager`. */ -abstract class InsecureTrustManagerSource extends DataFlow::Node { } +abstract class InsecureTrustManagerSource extends ApiSourceNode { } private class DefaultInsecureTrustManagerSource extends InsecureTrustManagerSource { DefaultInsecureTrustManagerSource() { @@ -18,7 +19,7 @@ private class DefaultInsecureTrustManagerSource extends InsecureTrustManagerSour * The use of a `TrustManager` in an SSL context. * Intentionally insecure connections are not considered sinks. */ -abstract class InsecureTrustManagerSink extends DataFlow::Node { +abstract class InsecureTrustManagerSink extends ApiSinkNode { InsecureTrustManagerSink() { not isGuardedByInsecureFlag(this) } } diff --git a/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulation.qll b/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulation.qll index 4309af8b3c8..2f9470f2bb9 100644 --- a/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulation.qll +++ b/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulation.qll @@ -6,6 +6,7 @@ import java private import semmle.code.java.controlflow.Guards private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.frameworks.android.Android private import semmle.code.java.frameworks.android.Intent @@ -14,7 +15,7 @@ private import semmle.code.java.frameworks.android.Intent * A sink for Intent URI permission manipulation vulnerabilities in Android, * that is, method calls that return an Intent as the result of an Activity. */ -abstract class IntentUriPermissionManipulationSink extends DataFlow::Node { } +abstract class IntentUriPermissionManipulationSink extends ApiSinkNode { } /** * A sanitizer that makes sure that an Intent is safe to be returned to another Activity. diff --git a/java/ql/lib/semmle/code/java/security/JWT.qll b/java/ql/lib/semmle/code/java/security/JWT.qll index 183495d8565..c84ebffabdb 100644 --- a/java/ql/lib/semmle/code/java/security/JWT.qll +++ b/java/ql/lib/semmle/code/java/security/JWT.qll @@ -2,9 +2,11 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks +private import semmle.code.java.dataflow.FlowSources /** A method access that assigns signing keys to a JWT parser. */ -class JwtParserWithInsecureParseSource extends DataFlow::Node { +class JwtParserWithInsecureParseSource extends ApiSourceNode { JwtParserWithInsecureParseSource() { exists(MethodCall ma, Method m | m.getDeclaringType().getAnAncestor() instanceof TypeJwtParser or @@ -24,7 +26,7 @@ class JwtParserWithInsecureParseSource extends DataFlow::Node { * the qualifier of a call to a `parse(token, handler)` method * where the `handler` is considered insecure. */ -class JwtParserWithInsecureParseSink extends DataFlow::Node { +class JwtParserWithInsecureParseSink extends ApiSinkNode { MethodCall insecureParseMa; JwtParserWithInsecureParseSink() { diff --git a/java/ql/lib/semmle/code/java/security/JndiInjection.qll b/java/ql/lib/semmle/code/java/security/JndiInjection.qll index d7282996057..3df8d6df378 100644 --- a/java/ql/lib/semmle/code/java/security/JndiInjection.qll +++ b/java/ql/lib/semmle/code/java/security/JndiInjection.qll @@ -3,11 +3,12 @@ import java private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.frameworks.Jndi private import semmle.code.java.frameworks.SpringLdap /** A data flow sink for unvalidated user input that is used in JNDI lookup. */ -abstract class JndiInjectionSink extends DataFlow::Node { } +abstract class JndiInjectionSink extends ApiSinkNode { } /** A sanitizer for JNDI injection vulnerabilities. */ abstract class JndiInjectionSanitizer extends DataFlow::Node { } diff --git a/java/ql/lib/semmle/code/java/security/OgnlInjection.qll b/java/ql/lib/semmle/code/java/security/OgnlInjection.qll index d5297702bef..37f31618fc3 100644 --- a/java/ql/lib/semmle/code/java/security/OgnlInjection.qll +++ b/java/ql/lib/semmle/code/java/security/OgnlInjection.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.frameworks.MyBatis @@ -10,7 +11,7 @@ private import semmle.code.java.frameworks.MyBatis * * Extend this class to add your own OGNL injection sinks. */ -abstract class OgnlInjectionSink extends DataFlow::Node { } +abstract class OgnlInjectionSink extends ApiSinkNode { } /** * A unit class for adding additional taint steps. diff --git a/java/ql/lib/semmle/code/java/security/QueryInjection.qll b/java/ql/lib/semmle/code/java/security/QueryInjection.qll index aa92aa16a14..df316155ba1 100644 --- a/java/ql/lib/semmle/code/java/security/QueryInjection.qll +++ b/java/ql/lib/semmle/code/java/security/QueryInjection.qll @@ -5,9 +5,10 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.frameworks.javaee.Persistence private import semmle.code.java.frameworks.MyBatis private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks /** A sink for database query language injection vulnerabilities. */ -abstract class QueryInjectionSink extends DataFlow::Node { } +abstract class QueryInjectionSink extends ApiSinkNode { } /** * A unit class for adding additional taint steps. diff --git a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll index 13a4b562a50..c0179860a01 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.SensitiveActions +private import semmle.code.java.dataflow.FlowSinks private class ResultReceiverSendCall extends MethodCall { ResultReceiverSendCall() { @@ -53,7 +54,7 @@ deprecated private class SensitiveResultReceiverConf extends TaintTracking::Conf /** * A class of sensitive result receiver sink nodes. */ -class SensitiveResultReceiverSink extends DataFlow::Node { +class SensitiveResultReceiverSink extends ApiSinkNode { SensitiveResultReceiverSink() { exists(ResultReceiverSendCall call | untrustedResultReceiverSend(_, call) and diff --git a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll index 884ab40a323..f9ff3f24040 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.SensitiveActions private import semmle.code.java.frameworks.android.Layout @@ -54,9 +55,9 @@ private class MaskCall extends MethodCall { } /** - * A class of test field sink nodes. + * A class of text field sink nodes. */ -class TextFieldSink extends DataFlow::Node { +class TextFieldSink extends ApiSinkNode { TextFieldSink() { exists(SetTextCall call | this.asExpr() = call.getStringArgument() and diff --git a/java/ql/lib/semmle/code/java/security/SpelInjection.qll b/java/ql/lib/semmle/code/java/security/SpelInjection.qll index 1aed2049afe..13eb195eae4 100644 --- a/java/ql/lib/semmle/code/java/security/SpelInjection.qll +++ b/java/ql/lib/semmle/code/java/security/SpelInjection.qll @@ -2,10 +2,11 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.frameworks.spring.SpringExpression /** A data flow sink for unvalidated user input that is used to construct SpEL expressions. */ -abstract class SpelExpressionEvaluationSink extends DataFlow::ExprNode { } +abstract class SpelExpressionEvaluationSink extends ApiSinkNode, DataFlow::ExprNode { } /** * A unit class for adding additional taint steps. diff --git a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll index 2e4b31b7785..5de0b0098e9 100644 --- a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.InformationLeak @@ -98,7 +99,7 @@ predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stac /** * A class of get message source nodes. */ -class GetMessageFlowSource extends DataFlow::Node { +class GetMessageFlowSource extends ApiSourceNode { GetMessageFlowSource() { exists(Method method | this.asExpr().(MethodCall).getMethod() = method | method.hasName("getMessage") and diff --git a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll index 970363fe543..96db99fe1b4 100644 --- a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll @@ -1,6 +1,7 @@ /** Provides classes to reason about local information disclosure in a temporary directory. */ import java +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.os.OSCheck private import semmle.code.java.security.TempDirUtils @@ -156,7 +157,7 @@ module TempDirSystemGetPropertyToCreate = /** * A class of method file directory creation sink nodes. */ -class MethodFileDirectoryCreationSink extends DataFlow::Node { +class MethodFileDirectoryCreationSink extends ApiSinkNode { MethodFileDirectoryCreationSink() { exists(MethodCall ma | ma.getMethod() instanceof MethodFileDirectoryCreation | ma.getQualifier() = this.asExpr() diff --git a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll index 499475cff3e..afd3af221be 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll @@ -4,6 +4,7 @@ import java private import semmle.code.java.dataflow.DataFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.frameworks.android.WebView private import semmle.code.java.frameworks.kotlin.Kotlin @@ -12,7 +13,7 @@ private import semmle.code.java.frameworks.kotlin.Kotlin * * Extend this class to add your own Unsafe Resource Fetching sinks. */ -abstract class UrlResourceSink extends DataFlow::Node { +abstract class UrlResourceSink extends ApiSinkNode { /** * Gets a description of this vulnerability. */ diff --git a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll index 5537add5a2c..b19d06bbf88 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll @@ -1,13 +1,14 @@ /** Provides classes to reason about vulnerabilites related to content URIs. */ import java +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.frameworks.android.Android private import semmle.code.java.security.PathSanitizer private import semmle.code.java.security.Sanitizers /** A URI that gets resolved by a `ContentResolver`. */ -abstract class ContentUriResolutionSink extends DataFlow::Node { } +abstract class ContentUriResolutionSink extends ApiSinkNode { } /** A sanitizer for content URIs. */ abstract class ContentUriResolutionSanitizer extends DataFlow::Node { } diff --git a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll index 272c483f7a2..734ad4c89fe 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll @@ -3,6 +3,7 @@ */ import semmle.code.java.dataflow.FlowSources +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking2 private import semmle.code.java.dispatch.VirtualDispatch private import semmle.code.java.frameworks.Kryo @@ -235,7 +236,7 @@ predicate unsafeDeserialization(MethodCall ma, Expr sink) { } /** A sink for unsafe deserialization. */ -class UnsafeDeserializationSink extends DataFlow::ExprNode { +class UnsafeDeserializationSink extends ApiSinkNode, DataFlow::ExprNode { UnsafeDeserializationSink() { unsafeDeserialization(_, this.getExpr()) } /** Gets a call that triggers unsafe deserialization. */ diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll index e806905c167..02f66e3f0e9 100644 --- a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll +++ b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll @@ -2,14 +2,15 @@ import java import semmle.code.java.dataflow.DataFlow -private import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.frameworks.Servlets import semmle.code.java.frameworks.ApacheHttp +private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.frameworks.JaxWS private import semmle.code.java.security.RequestForgery /** A URL redirection sink. */ -abstract class UrlRedirectSink extends DataFlow::Node { } +abstract class UrlRedirectSink extends ApiSinkNode { } /** A URL redirection sanitizer. */ abstract class UrlRedirectSanitizer extends DataFlow::Node { } diff --git a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll index d2a21be95e0..c7fd51b1c36 100644 --- a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll +++ b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.controlflow.Guards import semmle.code.java.security.SecurityTests +private import semmle.code.java.dataflow.FlowSinks /** Holds if `ex` looks like a check that this is a debug build. */ private predicate isDebugCheck(Expr ex) { @@ -47,7 +48,7 @@ deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration { /** * A class of webview debug sink nodes. */ -class WebviewDebugSink extends DataFlow::Node { +class WebviewDebugSink extends ApiSinkNode { WebviewDebugSink() { exists(MethodCall ma | ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and diff --git a/java/ql/lib/semmle/code/java/security/XPath.qll b/java/ql/lib/semmle/code/java/security/XPath.qll index 573d6530b33..c2992fdc272 100644 --- a/java/ql/lib/semmle/code/java/security/XPath.qll +++ b/java/ql/lib/semmle/code/java/security/XPath.qll @@ -3,12 +3,13 @@ import java import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks /** * A sink that represents a method that interprets XPath expressions. * Extend this class to add your own XPath Injection sinks. */ -abstract class XPathInjectionSink extends DataFlow::Node { } +abstract class XPathInjectionSink extends ApiSinkNode { } /** A default sink representing methods susceptible to XPath Injection attacks. */ private class DefaultXPathInjectionSink extends XPathInjectionSink { diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index aa69e5e7865..daf025141f5 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -10,9 +10,11 @@ private import semmle.code.java.frameworks.hudson.Hudson import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources +private import semmle.code.java.dataflow.FlowSinks /** A sink that represent a method that outputs data without applying contextual output encoding. */ -abstract class XssSink extends DataFlow::Node { } +abstract class XssSink extends ApiSinkNode { } /** A sanitizer that neutralizes dangerous characters that can be used to perform a XSS attack. */ abstract class XssSanitizer extends DataFlow::Node { } @@ -108,7 +110,7 @@ class XssVulnerableWriterSource extends MethodCall { /** * A class of xss vulnerable writer source nodes. */ -class XssVulnerableWriterSourceNode extends DataFlow::Node { +class XssVulnerableWriterSourceNode extends ApiSourceNode { XssVulnerableWriterSourceNode() { this.asExpr() instanceof XssVulnerableWriterSource } } diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index f6f3b1bf27c..0ab889f7372 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -4,6 +4,7 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.PathSanitizer private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.security.PathCreation /** @@ -24,7 +25,7 @@ private class ArchiveEntryNameMethod extends Method { /** * A class of entry name method source nodes. */ -class ArchiveEntryNameMethodSource extends DataFlow::Node { +class ArchiveEntryNameMethodSource extends ApiSourceNode { ArchiveEntryNameMethodSource() { this.asExpr().(MethodCall).getMethod() instanceof ArchiveEntryNameMethod } From 8def1c2c137e6bedf306dff0f3769f55ac64f266 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 3 May 2024 11:09:34 +0200 Subject: [PATCH 6/7] Java: Address review comments and some other code quality improvements. --- .../semmle/code/java/dataflow/FlowSinks.qll | 2 +- .../AndroidSensitiveCommunicationQuery.qll | 4 ++-- .../CleartextStorageAndroidDatabaseQuery.qll | 8 ++++---- ...CleartextStorageAndroidFilesystemQuery.qll | 8 ++++---- .../security/CleartextStorageCookieQuery.qll | 8 ++++---- .../CleartextStorageSharedPrefsQuery.qll | 8 ++++---- .../ExternallyControlledFormatStringQuery.qll | 4 ++-- .../java/security/ImplicitPendingIntents.qll | 2 -- .../ImproperIntentVerificationQuery.qll | 14 +++----------- .../java/security/InsecureRandomnessQuery.qll | 3 ++- java/ql/lib/semmle/code/java/security/JWT.qll | 1 - .../security/SensitiveResultReceiverQuery.qll | 4 ++-- .../code/java/security/SensitiveUiQuery.qll | 4 ++-- .../java/security/StackTraceExposureQuery.qll | 6 ++---- ...TempDirLocalInformationDisclosureQuery.qll | 19 ++++++------------- .../security/WebviewDebuggingEnabledQuery.qll | 4 ++-- java/ql/lib/semmle/code/java/security/XSS.qll | 2 +- .../code/java/security/ZipSlipQuery.qll | 4 ++-- 18 files changed, 43 insertions(+), 62 deletions(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll index 3b7fd191779..72cd96f6745 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll @@ -11,7 +11,7 @@ private import semmle.code.java.dataflow.DataFlow abstract class ApiSinkNode extends DataFlow::Node { } /** - * Add all models as data sinks. + * Add all sink models as data sinks. */ private class ApiSinkNodeExternal extends ApiSinkNode { ApiSinkNodeExternal() { 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 607ced09b2c..a4f9713ac30 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll @@ -153,9 +153,9 @@ deprecated class SensitiveCommunicationConfig extends TaintTracking::Configurati } /** - * A class of sensitive communication sink nodes. + * A sensitive communication sink node. */ -class SensitiveCommunicationSink extends ApiSinkNode { +private class SensitiveCommunicationSink extends ApiSinkNode { SensitiveCommunicationSink() { isSensitiveBroadcastSink(this) or diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index b4162f2c695..5ee9248d9eb 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -99,16 +99,16 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodCall store) } /** - * A class of local database open method call source nodes. + * A local database open method call source node. */ -class LocalDatabaseOpenMethodCallSource extends ApiSourceNode { +private class LocalDatabaseOpenMethodCallSource extends ApiSourceNode { LocalDatabaseOpenMethodCallSource() { this.asExpr() instanceof LocalDatabaseOpenMethodCall } } /** - * A class of local database sink nodes. + * A local database sink node. */ -class LocalDatabaseSink extends ApiSinkNode { +private class LocalDatabaseSink extends ApiSinkNode { LocalDatabaseSink() { localDatabaseInput(this, _) or localDatabaseStore(this, _) } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index 8b1af7b4971..06fa8381312 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -82,16 +82,16 @@ private class CloseFileMethod extends Method { } /** - * A class of local file open call source nodes. + * A local file open call source node. */ -class LocalFileOpenCallSource extends ApiSourceNode { +private class LocalFileOpenCallSource extends ApiSourceNode { LocalFileOpenCallSource() { this.asExpr() instanceof LocalFileOpenCall } } /** - * A class of local file sink nodes. + * A local file sink node. */ -class LocalFileSink extends ApiSinkNode { +private class LocalFileSink extends ApiSinkNode { LocalFileSink() { filesystemInput(this, _) or closesFile(this, _) diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index c3684646bdd..a36a4754584 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -40,16 +40,16 @@ private predicate cookieStore(DataFlow::Node cookie, Expr store) { } /** - * A class of cookie source nodes. + * A cookie source node. */ -class CookieSource extends ApiSourceNode { +private class CookieSource extends ApiSourceNode { CookieSource() { this.asExpr() instanceof Cookie } } /** - * A class of cookie store sink nodes. + * A cookie store sink node. */ -class CookieStoreSink extends ApiSinkNode { +private class CookieStoreSink extends ApiSinkNode { CookieStoreSink() { cookieStore(this, _) } } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index 80dc2fca1f4..f72d40106e3 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -70,18 +70,18 @@ private predicate sharedPreferencesStore(DataFlow::Node editor, MethodCall m) { } /** - * A shared preferences editor method call source nodes. + * A shared preferences editor method call source node. */ -class SharedPreferencesEditorMethodCallSource extends ApiSourceNode { +private class SharedPreferencesEditorMethodCallSource extends ApiSourceNode { SharedPreferencesEditorMethodCallSource() { this.asExpr() instanceof SharedPreferencesEditorMethodCall } } /** - * A class of shared preferences sink nodes. + * A shared preferences sink node. */ -class SharedPreferencesSink extends ApiSinkNode { +private class SharedPreferencesSink extends ApiSinkNode { SharedPreferencesSink() { sharedPreferencesInput(this, _) or sharedPreferencesStore(this, _) diff --git a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll index 8d6fe0426c3..606e31a07cb 100644 --- a/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ExternallyControlledFormatStringQuery.qll @@ -6,9 +6,9 @@ private import semmle.code.java.dataflow.FlowSources private import semmle.code.java.StringFormat /** - * A class of string format sink nodes. + * A string format sink node. */ -class StringFormatSink extends ApiSinkNode { +private class StringFormatSink extends ApiSinkNode { StringFormatSink() { this.asExpr() = any(StringFormat formatCall).getFormatArgument() } } diff --git a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll index 5c4094de3d3..a5d8f256b03 100644 --- a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll +++ b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll @@ -3,8 +3,6 @@ import java private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSources -private import semmle.code.java.dataflow.TaintTracking -private import semmle.code.java.frameworks.android.Intent private import semmle.code.java.frameworks.android.PendingIntent private newtype TPendingIntentState = diff --git a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll index e8bfc97b0fc..bca045bc8e4 100644 --- a/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll @@ -4,7 +4,6 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.xml.AndroidManifest import semmle.code.java.frameworks.android.Intent -private import semmle.code.java.dataflow.FlowSources /** An `onReceive` method of a `BroadcastReceiver` */ private class OnReceiveMethod extends Method { @@ -14,18 +13,11 @@ private class OnReceiveMethod extends Method { Parameter getIntentParameter() { result = this.getParameter(1) } } -/** - * A class of verified intent source nodes. - */ -class VerifiedIntentConfigSource extends ApiSourceNode { - VerifiedIntentConfigSource() { - this.asParameter() = any(OnReceiveMethod orm).getIntentParameter() - } -} - /** A configuration to detect whether the `action` of an `Intent` is checked. */ private module VerifiedIntentConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node src) { src instanceof VerifiedIntentConfigSource } + predicate isSource(DataFlow::Node src) { + src.asParameter() = any(OnReceiveMethod orm).getIntentParameter() + } predicate isSink(DataFlow::Node sink) { exists(MethodCall ma | diff --git a/java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll b/java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll index f983876d7b3..423046b6746 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureRandomnessQuery.qll @@ -4,6 +4,7 @@ import java private import semmle.code.java.frameworks.OpenSaml private import semmle.code.java.frameworks.Servlets private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.Cookies private import semmle.code.java.security.RandomQuery @@ -49,7 +50,7 @@ abstract class InsecureRandomnessSink extends DataFlow::Node { } /** * A node which sets the value of a cookie. */ -private class CookieSink extends InsecureRandomnessSink { +private class CookieSink extends InsecureRandomnessSink, ApiSinkNode { CookieSink() { this.asExpr() instanceof SetCookieValue } } diff --git a/java/ql/lib/semmle/code/java/security/JWT.qll b/java/ql/lib/semmle/code/java/security/JWT.qll index c84ebffabdb..5ba47072dc6 100644 --- a/java/ql/lib/semmle/code/java/security/JWT.qll +++ b/java/ql/lib/semmle/code/java/security/JWT.qll @@ -1,7 +1,6 @@ /** Provides classes for working with JSON Web Token (JWT) libraries. */ import java -private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.FlowSinks private import semmle.code.java.dataflow.FlowSources diff --git a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll index c0179860a01..8269a42c5c2 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveResultReceiverQuery.qll @@ -52,9 +52,9 @@ deprecated private class SensitiveResultReceiverConf extends TaintTracking::Conf } /** - * A class of sensitive result receiver sink nodes. + * A sensitive result receiver sink node. */ -class SensitiveResultReceiverSink extends ApiSinkNode { +private class SensitiveResultReceiverSink extends ApiSinkNode { SensitiveResultReceiverSink() { exists(ResultReceiverSendCall call | untrustedResultReceiverSend(_, call) and diff --git a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll index f9ff3f24040..a7e76d0e2e3 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll @@ -55,9 +55,9 @@ private class MaskCall extends MethodCall { } /** - * A class of text field sink nodes. + * A text field sink node. */ -class TextFieldSink extends ApiSinkNode { +private class TextFieldSink extends ApiSinkNode { TextFieldSink() { exists(SetTextCall call | this.asExpr() = call.getStringArgument() and diff --git a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll index 5de0b0098e9..1a2fe31e879 100644 --- a/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll @@ -1,9 +1,7 @@ /** Provides predicates to reason about exposure of stack-traces. */ import java -private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.FlowSources -private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.InformationLeak /** @@ -97,9 +95,9 @@ predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stac } /** - * A class of get message source nodes. + * A get message source node. */ -class GetMessageFlowSource extends ApiSourceNode { +private class GetMessageFlowSource extends ApiSourceNode { GetMessageFlowSource() { exists(Method method | this.asExpr().(MethodCall).getMethod() = method | method.hasName("getMessage") and diff --git a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll index 96db99fe1b4..f1ffcaecc51 100644 --- a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll +++ b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll @@ -30,7 +30,7 @@ private class MethodFileFileCreation extends MethodFileSystemFileCreation { /** * A dataflow node that creates a file or directory in the file system. */ -abstract private class FileCreationSink extends DataFlow::Node { } +abstract private class FileCreationSink extends ApiSinkNode { } /** * The qualifier of a call to one of `File`'s file-creating or directory-creating methods, @@ -154,17 +154,6 @@ module TempDirSystemGetPropertyToCreateConfig implements DataFlow::ConfigSig { module TempDirSystemGetPropertyToCreate = TaintTracking::Global; -/** - * A class of method file directory creation sink nodes. - */ -class MethodFileDirectoryCreationSink extends ApiSinkNode { - 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: @@ -184,7 +173,11 @@ module TempDirSystemGetPropertyDirectlyToMkdirConfig implements DataFlow::Config ) } - predicate isSink(DataFlow::Node node) { node instanceof MethodFileDirectoryCreationSink } + predicate isSink(DataFlow::Node node) { + exists(MethodCall ma | ma.getMethod() instanceof MethodFileDirectoryCreation | + ma.getQualifier() = node.asExpr() + ) + } 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 c7fd51b1c36..f10b0132b5a 100644 --- a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll +++ b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll @@ -46,9 +46,9 @@ deprecated class WebviewDebugEnabledConfig extends DataFlow::Configuration { } /** - * A class of webview debug sink nodes. + * A webview debug sink node. */ -class WebviewDebugSink extends ApiSinkNode { +private class WebviewDebugSink extends ApiSinkNode { WebviewDebugSink() { exists(MethodCall ma | ma.getMethod().hasQualifiedName("android.webkit", "WebView", "setWebContentsDebuggingEnabled") and diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll index daf025141f5..777e5fae062 100644 --- a/java/ql/lib/semmle/code/java/security/XSS.qll +++ b/java/ql/lib/semmle/code/java/security/XSS.qll @@ -108,7 +108,7 @@ class XssVulnerableWriterSource extends MethodCall { } /** - * A class of xss vulnerable writer source nodes. + * A xss vulnerable writer source node. */ class XssVulnerableWriterSourceNode extends ApiSourceNode { XssVulnerableWriterSourceNode() { this.asExpr() instanceof XssVulnerableWriterSource } diff --git a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll index 0ab889f7372..75e2f7000c5 100644 --- a/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll @@ -23,9 +23,9 @@ private class ArchiveEntryNameMethod extends Method { } /** - * A class of entry name method source nodes. + * An entry name method source node. */ -class ArchiveEntryNameMethodSource extends ApiSourceNode { +private class ArchiveEntryNameMethodSource extends ApiSourceNode { ArchiveEntryNameMethodSource() { this.asExpr().(MethodCall).getMethod() instanceof ArchiveEntryNameMethod } From c07bf65eb6cf1dc7e29b9c467c8ffd5552d9675a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 3 May 2024 11:13:05 +0200 Subject: [PATCH 7/7] Update java/ql/lib/semmle/code/java/dataflow/FlowSources.qll Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> --- java/ql/lib/semmle/code/java/dataflow/FlowSources.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll index f28cc998487..6befe289a17 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll @@ -397,7 +397,7 @@ abstract class ApiSourceNode extends DataFlow::Node { } private class AddSourceNodes extends ApiSourceNode instanceof SourceNode { } /** - * Add all models as data sources. + * Add all source models as data sources. */ private class ApiSourceNodeExternal extends ApiSourceNode { ApiSourceNodeExternal() { sourceNode(this, _) }