Skip to content

Commit

Permalink
Java: Improve the Api sinks implementation.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelnebel committed Apr 26, 2024
1 parent 1308759 commit 5f660c4
Show file tree
Hide file tree
Showing 28 changed files with 96 additions and 136 deletions.
138 changes: 32 additions & 106 deletions java/ql/lib/semmle/code/java/dataflow/ApiSinks.qll
Expand Up @@ -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, _) }
}
}
8 changes: 8 additions & 0 deletions java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll
@@ -0,0 +1,8 @@
private import java

Check warning on line 1 in java/ql/lib/semmle/code/java/dataflow/FlowSinks.qll

View workflow job for this annotation

GitHub Actions / qldoc

Missing QLdoc for file FlowSinks
private import semmle.code.java.dataflow.DataFlow

/**
* A data flow sink node for an API, which should be considered
* supported for a modelling perspective.
*/

Check warning

Code scanning / CodeQL

Misspelling Warning

This comment contains the non-US spelling 'modelling', which should instead be 'modeling'.
abstract class ApiSinkNode extends DataFlow::Node { }

Check failure

Code scanning / CodeQL

Bidirectional imports for abstract classes Error

This abstract class doesn't import its subclass
ApiSinkNodeExternal
but imports 41 other subclasses, such as
JaxRSXssSink
.
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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. */
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, _) }
}

Expand Down
Expand Up @@ -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() {
Expand Down Expand Up @@ -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, _)
Expand Down
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, _) }
}

Expand Down
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, _)
Expand Down
@@ -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() }
}

Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/security/InsecureBasicAuth.qll
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/security/InsecureLdapAuth.qll
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
@@ -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

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.java.security.SecurityFlag
.
private import semmle.code.java.security.Encryption
private import semmle.code.java.security.SecurityFlag

Expand All @@ -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) }
}

Expand Down
Expand Up @@ -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
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/security/JWT.qll
Expand Up @@ -2,6 +2,7 @@

import java
private import semmle.code.java.dataflow.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.java.dataflow.FlowSources
.
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. */
Expand All @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/security/JndiInjection.qll
Expand Up @@ -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 { }
Expand Down

0 comments on commit 5f660c4

Please sign in to comment.