Skip to content

Commit

Permalink
Java: Address review comments and some other code quality improvements.
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelnebel committed May 3, 2024
1 parent f95b330 commit 5245fd8
Show file tree
Hide file tree
Showing 17 changed files with 42 additions and 61 deletions.
Expand Up @@ -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
Expand Down
Expand Up @@ -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, _) }
}

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

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

Expand Down
Expand Up @@ -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 =
Expand Down
Expand Up @@ -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 {
Expand All @@ -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 |
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}

Expand Down
1 change: 0 additions & 1 deletion 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

Expand Down
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/security/SensitiveUiQuery.qll
Expand Up @@ -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
Expand Down
@@ -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

/**
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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,
Expand Down Expand Up @@ -154,17 +154,6 @@ module TempDirSystemGetPropertyToCreateConfig implements DataFlow::ConfigSig {
module TempDirSystemGetPropertyToCreate =
TaintTracking::Global<TempDirSystemGetPropertyToCreateConfig>;

/**
* 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:
Expand All @@ -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(), _, _)
Expand Down
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion java/ql/lib/semmle/code/java/security/XSS.qll
Expand Up @@ -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 }
Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll
Expand Up @@ -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
}
Expand Down

0 comments on commit 5245fd8

Please sign in to comment.