Java: Improve API sinks implementation.

michaelnebel · michaelnebel · commit 79f54fb0926e · 2024-04-26T12:32:52.000+02:00
diff --git a/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll b/java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll
@@ -2,17 +2,15 @@
 
 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 { }
+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 module AllApiSinks {
   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
@@ -41,82 +39,12 @@ private module ApiSinks {
   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 class ApiSinkNodeExternal extends SinkNode {
+    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
@@ -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
@@ -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
diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll
@@ -6,6 +6,7 @@ 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 {
@@ -107,7 +108,7 @@ class LocalDatabaseOpenMethodCallSource extends ApiSourceNode {
 /**
  * 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
@@ -5,10 +5,11 @@
 
 import java
 import semmle.code.java.dataflow.DataFlow
-private import semmle.code.java.dataflow.ExternalFlow
-private import semmle.code.java.dataflow.FlowSources
 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() {
@@ -90,7 +91,7 @@ class LocalFileOpenCallSource extends ApiSourceNode {
 /**
  * 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
@@ -4,6 +4,7 @@ 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 {
@@ -48,7 +49,7 @@ class CookieSource extends ApiSourceNode {
 /**
  * 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
@@ -4,6 +4,7 @@ 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 {
@@ -80,7 +81,7 @@ class SharedPreferencesEditorMethodCallSource extends ApiSourceNode {
 /**
  * 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
@@ -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/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
@@ -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
@@ -1,8 +1,9 @@
 /** Provides classes and predicates to reason about insecure `TrustManager`s. */
 
 import java
-private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.controlflow.Guards
+private import semmle.code.java.dataflow.FlowSinks
+private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.security.Encryption
 private import semmle.code.java.security.SecurityFlag
 
@@ -19,7 +20,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
@@ -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
@@ -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.FlowSources
 
 /** A method access that assigns signing keys to a JWT parser. */
@@ -25,7 +26,7 @@ class JwtParserWithInsecureParseSource extends ApiSourceNode {
  * 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -56,7 +57,7 @@ private class MaskCall extends MethodCall {
 /**
  * A class of test 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
diff --git a/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll b/java/ql/lib/semmle/code/java/security/TempDirLocalInformationDisclosureQuery.qll
diff --git a/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll b/java/ql/lib/semmle/code/java/security/UnsafeAndroidAccess.qll
diff --git a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolution.qll
diff --git a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll
diff --git a/java/ql/lib/semmle/code/java/security/UrlRedirect.qll b/java/ql/lib/semmle/code/java/security/UrlRedirect.qll
diff --git a/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll b/java/ql/lib/semmle/code/java/security/WebviewDebuggingEnabledQuery.qll
diff --git a/java/ql/lib/semmle/code/java/security/XPath.qll b/java/ql/lib/semmle/code/java/security/XPath.qll
diff --git a/java/ql/lib/semmle/code/java/security/XSS.qll b/java/ql/lib/semmle/code/java/security/XSS.qll
