Java: Improve the Api sinks implementation.

Author: michaelnebel

java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll

@@ -2,121 +2,47 @@
 
 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 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 { }
+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
 
   /**
    * Add all models as data sinks.
    */
-  private class SinkNodeExternal extends SinkNode {
-    SinkNodeExternal() { sinkNode(this, _) }
+  private class ApiSinkNodeExternal extends SinkNode {
+    ApiSinkNodeExternal() { sinkNode(this, _) }
   }
 }

java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll

@@ -0,0 +1,8 @@
+private import java
+private import semmle.code.java.dataflow.DataFlow
+
+/**
+ * A data flow sink node for an API, which should be considered
+ * supported for a modelling perspective.
+ */
+abstract class ApiSinkNode extends DataFlow::Node { }

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

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

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, _) }
 }
 

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, _)

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, _) }
 }
 

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, _)

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() }
 }
 

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 {