From b8dd0472f7d20c57432ba9f3b605906a1cba43c0 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 22 Apr 2024 11:21:13 +0200 Subject: [PATCH] Java: Indentify more APIs as supported in the telemetry queries (as QL defined sources). --- .../semmle/code/java/dataflow/ApiSources.qll | 73 +++++++++++++++++++ .../semmle/code/java/dataflow/FlowSources.qll | 8 +- .../CleartextStorageAndroidDatabaseQuery.qll | 11 ++- ...CleartextStorageAndroidFilesystemQuery.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 +- 10 files changed, 139 insertions(+), 22 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 0000000000000..7554767b270da --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/ApiSources.qll @@ -0,0 +1,73 @@ +/** Provides classes representing various flow sources for data flow / taint tracking. */ + +private import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.FlowSources + +/** + * A data flow source node. + */ +abstract class SourceNode extends DataFlow::Node { } + +/** + * Module that adds all API like sources to `SourceNode`, excluding sources for cryptography based + * queries, and queries where sources are not succifiently. + */ +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.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.InsecureTrustManagerQuery as InsecureTrustManagerQuery + private import semmle.code.java.security.MissingJWTSignatureCheckQuery as MissingJWTSignatureCheckQuery + private import semmle.code.java.security.XSS as Xss + private import semmle.code.java.security.StackTraceExposureQuery as StackTraceExposureQuery + private import semmle.code.java.security.UnsafeCertTrustQuery as UnsafeCertTrustQuery + 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 CleartextStorageSharedPrefsQuerySources extends SourceNode instanceof CleartextStorageSharedPrefsQuery::SharedPreferencesEditorMethodCallSource + { } + + private class ImplicitPendingIntentsQuerySources extends SourceNode instanceof ImplicitPendingIntentsQuery::ImplicitPendingIntentSource + { } + + private class ImproperIntentVerificationQuerySources extends SourceNode instanceof ImproperIntentVerificationQuery::VerifiedIntentConfigSource + { } + + private class InsecureTrustManagerQuerySources extends SourceNode instanceof InsecureTrustManagerQuery::InsecureTrustManagerSource + { } + + private class MissingJWTSignatureCheckQuerySources extends SourceNode instanceof MissingJWTSignatureCheckQuery::JwtParserWithInsecureParseSource + { } + + private class XssSources extends SourceNode instanceof Xss::XssVulnerableWriterSourceNode { } + + private class StackTraceExposureQuerySources extends SourceNode instanceof StackTraceExposureQuery::GetMessageFlowSource + { } + + private class UnsafeCertTrustQuerySources extends SourceNode instanceof UnsafeCertTrustQuery::SslConnectionInit + { } + + 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 425eb3ccaa608..af206f95420b1 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 = 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 b42389a1df6ec..7f306298a45eb 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 d7097f1ecf23f..0759b4c061b08 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/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index 39e1ffa3c75dc..3f690aeb6f19b 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 bca045bc8e422..92bcac5b50e0c 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 dba9492d137bb..2e4b31b7785ea 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 9edee5823bfc7..aa69e5e7865f7 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 7ba99a31e2681..f6f3b1bf27c40 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 47527dfbb46a1..cbf2875e9d1fc 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]