diff --git a/java/ql/lib/semmle/code/java/frameworks/JsonIo.qll b/java/ql/lib/semmle/code/java/frameworks/JsonIo.qll index ab4c1b115d9..aa332bd78ed 100644 --- a/java/ql/lib/semmle/code/java/frameworks/JsonIo.qll +++ b/java/ql/lib/semmle/code/java/frameworks/JsonIo.qll @@ -42,8 +42,12 @@ class JsonIoUseMapsSetter extends MethodAccess { } } -/** A data flow configuration tracing flow from JsonIo safe settings. */ -class SafeJsonIoConfig extends DataFlow2::Configuration { +/** + * DEPRECATED: Use `SafeJsonIoFlow` instead. + * + * A data flow configuration tracing flow from JsonIo safe settings. + */ +deprecated class SafeJsonIoConfig extends DataFlow2::Configuration { SafeJsonIoConfig() { this = "UnsafeDeserialization::SafeJsonIoConfig" } override predicate isSource(DataFlow::Node src) { @@ -65,3 +69,30 @@ class SafeJsonIoConfig extends DataFlow2::Configuration { ) } } + +/** + * A data flow configuration tracing flow from JsonIo safe settings. + */ +module SafeJsonIoConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { + exists(MethodAccess ma | + ma instanceof JsonIoUseMapsSetter and + src.asExpr() = ma.getQualifier() + ) + } + + predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + ma.getMethod() instanceof JsonIoJsonToJavaMethod and + sink.asExpr() = ma.getArgument(1) + ) + or + exists(ClassInstanceExpr cie | + cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and + sink.asExpr() = cie.getArgument(1) + ) + } +} + +/** Tracks flow from JsonIo safe settings. */ +module SafeJsonIoFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll b/java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll index 3ea5d79c138..f21b9552061 100644 --- a/java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll +++ b/java/ql/lib/semmle/code/java/frameworks/SnakeYaml.qll @@ -4,8 +4,6 @@ import java import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.DataFlow2 -import semmle.code.java.dataflow.DataFlow3 /** * The class `org.yaml.snakeyaml.constructor.SafeConstructor`. @@ -30,28 +28,28 @@ class Yaml extends RefType { Yaml() { this.getAnAncestor().hasQualifiedName("org.yaml.snakeyaml", "Yaml") } } -private class SafeYamlConstructionFlowConfig extends DataFlow3::Configuration { - SafeYamlConstructionFlowConfig() { this = "SnakeYaml::SafeYamlConstructionFlowConfig" } - - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof SafeSnakeYamlConstruction - } - - override predicate isSink(DataFlow::Node sink) { sink = this.yamlClassInstanceExprArgument(_) } - - private DataFlow::ExprNode yamlClassInstanceExprArgument(ClassInstanceExpr cie) { - cie.getConstructedType() instanceof Yaml and - result.getExpr() = cie.getArgument(0) - } - - ClassInstanceExpr getSafeYaml() { this.hasFlowTo(this.yamlClassInstanceExprArgument(result)) } +private DataFlow::ExprNode yamlClassInstanceExprArgument(ClassInstanceExpr cie) { + cie.getConstructedType() instanceof Yaml and + result.getExpr() = cie.getArgument(0) } +private module SafeYamlConstructionFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSnakeYamlConstruction } + + predicate isSink(DataFlow::Node sink) { sink = yamlClassInstanceExprArgument(_) } + + additional ClassInstanceExpr getSafeYaml() { + SafeYamlConstructionFlow::flowTo(yamlClassInstanceExprArgument(result)) + } +} + +private module SafeYamlConstructionFlow = DataFlow::Global; + /** * An instance of `Yaml` that does not allow arbitrary constructor to be called. */ private class SafeYaml extends ClassInstanceExpr { - SafeYaml() { exists(SafeYamlConstructionFlowConfig conf | conf.getSafeYaml() = this) } + SafeYaml() { SafeYamlConstructionFlowConfig::getSafeYaml() = this } } /** A call to a parse method of `Yaml`. */ @@ -65,23 +63,25 @@ private class SnakeYamlParse extends MethodAccess { } } -private class SafeYamlFlowConfig extends DataFlow2::Configuration { - SafeYamlFlowConfig() { this = "SnakeYaml::SafeYamlFlowConfig" } +private module SafeYamlFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeYaml } + predicate isSink(DataFlow::Node sink) { sink = yamlParseQualifier(_) } - override predicate isSink(DataFlow::Node sink) { sink = this.yamlParseQualifier(_) } - - private DataFlow::ExprNode yamlParseQualifier(SnakeYamlParse syp) { + additional DataFlow::ExprNode yamlParseQualifier(SnakeYamlParse syp) { result.getExpr() = syp.getQualifier() } - SnakeYamlParse getASafeSnakeYamlParse() { this.hasFlowTo(this.yamlParseQualifier(result)) } + additional SnakeYamlParse getASafeSnakeYamlParse() { + SafeYamlFlow::flowTo(yamlParseQualifier(result)) + } } +private module SafeYamlFlow = DataFlow::Global; + /** * A call to a parse method of `Yaml` that allows arbitrary constructor to be called. */ class UnsafeSnakeYamlParse extends SnakeYamlParse { - UnsafeSnakeYamlParse() { not exists(SafeYamlFlowConfig sy | sy.getASafeSnakeYamlParse() = this) } + UnsafeSnakeYamlParse() { not SafeYamlFlowConfig::getASafeSnakeYamlParse() = this } } diff --git a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll index 5a913ccdef8..f517d6dec64 100644 --- a/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll +++ b/java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll @@ -136,24 +136,22 @@ private class GuavaRegexFlowStep extends RegexAdditionalFlowStep { } } -private class RegexFlowConf extends DataFlow2::Configuration { - RegexFlowConf() { this = "RegexFlowConfig" } +private module RegexFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node.asExpr() instanceof ExploitableStringLiteral } - override predicate isSource(DataFlow::Node node) { - node.asExpr() instanceof ExploitableStringLiteral - } + predicate isSink(DataFlow::Node node) { node instanceof RegexFlowSink } - override predicate isSink(DataFlow::Node node) { node instanceof RegexFlowSink } - - override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { any(RegexAdditionalFlowStep s).step(node1, node2) } - override predicate isBarrier(DataFlow::Node node) { + predicate isBarrier(DataFlow::Node node) { node.getEnclosingCallable().getDeclaringType() instanceof NonSecurityTestClass } } +private module RegexFlow = DataFlow::Global; + /** * Holds if `regex` is used as a regex, with the mode `mode` (if known). * If regex mode is not known, `mode` will be `"None"`. @@ -162,7 +160,7 @@ private class RegexFlowConf extends DataFlow2::Configuration { * and therefore may be relevant for ReDoS queries are considered. */ predicate usedAsRegex(StringLiteral regex, string mode, boolean match_full_string) { - any(RegexFlowConf c).hasFlow(DataFlow2::exprNode(regex), _) and + RegexFlow::flow(DataFlow::exprNode(regex), _) and mode = "None" and // TODO: proper mode detection (if matchesFullString(regex) then match_full_string = true else match_full_string = false) } @@ -172,9 +170,9 @@ predicate usedAsRegex(StringLiteral regex, string mode, boolean match_full_strin * as though it was implicitly surrounded by ^ and $. */ private predicate matchesFullString(StringLiteral regex) { - exists(RegexFlowConf c, RegexFlowSink sink | + exists(RegexFlowSink sink | sink.matchesFullString() and - c.hasFlow(DataFlow2::exprNode(regex), sink) + RegexFlow::flow(DataFlow::exprNode(regex), sink) ) } @@ -185,8 +183,8 @@ private predicate matchesFullString(StringLiteral regex) { * and therefore may be relevant for ReDoS queries are considered. */ predicate regexMatchedAgainst(StringLiteral regex, Expr str) { - exists(RegexFlowConf c, RegexFlowSink sink | + exists(RegexFlowSink sink | str = sink.getStringArgument() and - c.hasFlow(DataFlow2::exprNode(regex), sink) + RegexFlow::flow(DataFlow::exprNode(regex), sink) ) } diff --git a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll index e0c9fbff800..e8d5a6d42d1 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll @@ -151,7 +151,10 @@ deprecated class SensitiveCommunicationConfig extends TaintTracking::Configurati } } -private module SensitiveCommunicationConfig implements DataFlow::ConfigSig { +/** + * Taint configuration tracking flow from variables containing sensitive information to broadcast Intents. + */ +module SensitiveCommunicationConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveInfoExpr } predicate isSink(DataFlow::Node sink) { diff --git a/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll b/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll index d066f4974a1..9df15067be9 100644 --- a/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll @@ -9,7 +9,7 @@ private import semmle.code.java.security.ArbitraryApkInstallation * A dataflow configuration for flow from an external source of an APK to the * `setData[AndType][AndNormalize]` method of an intent. */ -private module ApkInstallationConfig implements DataFlow::ConfigSig { +module ApkInstallationConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node node) { node instanceof ExternalApkSource } predicate isSink(DataFlow::Node node) { diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll index 7edd9e74c50..d995d11ce5b 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidDatabaseQuery.qll @@ -29,16 +29,16 @@ class LocalDatabaseOpenMethodAccess extends Storable, Call { } override Expr getAnInput() { - exists(LocalDatabaseFlowConfig config, DataFlow::Node database | + exists(DataFlow::Node database | localDatabaseInput(database, result) and - config.hasFlow(DataFlow::exprNode(this), database) + LocalDatabaseFlow::flow(DataFlow::exprNode(this), database) ) } override Expr getAStore() { - exists(LocalDatabaseFlowConfig config, DataFlow::Node database | + exists(DataFlow::Node database | localDatabaseStore(database, result) and - config.hasFlow(DataFlow::exprNode(this), database) + LocalDatabaseFlow::flow(DataFlow::exprNode(this), database) ) } } @@ -93,19 +93,17 @@ private predicate localDatabaseStore(DataFlow::Node database, MethodAccess store ) } -private class LocalDatabaseFlowConfig extends DataFlow::Configuration { - LocalDatabaseFlowConfig() { this = "LocalDatabaseFlowConfig" } - - override predicate isSource(DataFlow::Node source) { +private module LocalDatabaseFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof LocalDatabaseOpenMethodAccess } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { localDatabaseInput(sink, _) or localDatabaseStore(sink, _) } - override predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { // Adds a step for tracking databases through field flow, that is, a database is opened and // assigned to a field, and then an input or store method is called on that field elsewhere. exists(Field f | @@ -115,3 +113,5 @@ private class LocalDatabaseFlowConfig extends DataFlow::Configuration { ) } } + +private module LocalDatabaseFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll index 89cc7ac021b..2641a3ab0df 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageAndroidFilesystemQuery.qll @@ -24,16 +24,16 @@ class LocalFileOpenCall extends Storable { } override Expr getAnInput() { - exists(FilesystemFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | filesystemInput(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + FilesystemFlow::flow(DataFlow::exprNode(this), n) ) } override Expr getAStore() { - exists(FilesystemFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | closesFile(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + FilesystemFlow::flow(DataFlow::exprNode(this), n) ) } } @@ -79,17 +79,15 @@ private class CloseFileMethod extends Method { } } -private class FilesystemFlowConfig extends DataFlow::Configuration { - FilesystemFlowConfig() { this = "FilesystemFlowConfig" } +private module FilesystemFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LocalFileOpenCall } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { filesystemInput(sink, _) or closesFile(sink, _) } - override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { // Add nested Writer constructors as extra data flow steps exists(ClassInstanceExpr cie | cie.getConstructedType().getAnAncestor().hasQualifiedName("java.io", "Writer") and @@ -98,3 +96,5 @@ private class FilesystemFlowConfig extends DataFlow::Configuration { ) } } + +private module FilesystemFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll index 3fe06a2de08..41e5c060816 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageClassQuery.qll @@ -14,8 +14,8 @@ private class ClassCleartextStorageSink extends CleartextStorageSink { abstract class ClassStore extends Storable, ClassInstanceExpr { /** Gets an input, for example `input` in `instance.password = input`. */ override Expr getAnInput() { - exists(ClassStoreFlowConfig conf, DataFlow::Node instance | - conf.hasFlow(DataFlow::exprNode(this), instance) and + exists(DataFlow::Node instance | + ClassStoreFlow::flow(DataFlow::exprNode(this), instance) and result = getInstanceInput(instance, this.getConstructor().getDeclaringType()) ) } @@ -40,9 +40,9 @@ private class Serializable extends ClassStore { /** Gets a store, for example `outputStream.writeObject(instance)`. */ override Expr getAStore() { - exists(ClassStoreFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | serializableStore(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + ClassStoreFlow::flow(DataFlow::exprNode(this), n) ) } } @@ -53,9 +53,9 @@ private class Marshallable extends ClassStore { /** Gets a store, for example `marshaller.marshal(instance)`. */ override Expr getAStore() { - exists(ClassStoreFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | marshallableStore(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + ClassStoreFlow::flow(DataFlow::exprNode(this), n) ) } } @@ -73,20 +73,20 @@ private Expr getInstanceInput(DataFlow::Node instance, RefType t) { ) } -private class ClassStoreFlowConfig extends DataFlow::Configuration { - ClassStoreFlowConfig() { this = "ClassStoreFlowConfig" } +private module ClassStoreFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ClassStore } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(getInstanceInput(sink, _)) or serializableStore(sink, _) or marshallableStore(sink, _) } - override int fieldFlowBranchLimit() { result = 1 } + int fieldFlowBranchLimit() { result = 1 } } +private module ClassStoreFlow = DataFlow::Global; + private predicate serializableStore(DataFlow::Node instance, Expr store) { exists(MethodAccess m | store = m and diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll index bd071e7fef6..59c098281f1 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageCookieQuery.qll @@ -20,9 +20,9 @@ class Cookie extends Storable, ClassInstanceExpr { /** Gets a store, for example `response.addCookie(cookie);`. */ override Expr getAStore() { - exists(CookieToStoreFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | cookieStore(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + CookieToStoreFlow::flow(DataFlow::exprNode(this), n) ) } } @@ -37,12 +37,12 @@ private predicate cookieStore(DataFlow::Node cookie, Expr store) { ) } -private class CookieToStoreFlowConfig extends DataFlow3::Configuration { - CookieToStoreFlowConfig() { this = "CookieToStoreFlowConfig" } +private module CookieToStoreFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Cookie } - - override predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) } + predicate isSink(DataFlow::Node sink) { cookieStore(sink, _) } } +private module CookieToStoreFlow = DataFlow::Global; + private Expr cookieInput(Cookie c) { result = c.getArgument(1) } diff --git a/java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll index c75689c1fd2..f262104078e 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStoragePropertiesQuery.qll @@ -19,17 +19,17 @@ class Properties extends Storable, ClassInstanceExpr { /** Gets an input, for example `input` in `props.setProperty("password", input);`. */ override Expr getAnInput() { - exists(PropertiesFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | propertiesInput(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + PropertiesFlow::flow(DataFlow::exprNode(this), n) ) } /** Gets a store, for example `props.store(outputStream, "...")`. */ override Expr getAStore() { - exists(PropertiesFlowConfig conf, DataFlow::Node n | + exists(DataFlow::Node n | propertiesStore(n, result) and - conf.hasFlow(DataFlow::exprNode(this), n) + PropertiesFlow::flow(DataFlow::exprNode(this), n) ) } } @@ -50,13 +50,13 @@ private predicate propertiesStore(DataFlow::Node prop, Expr store) { ) } -private class PropertiesFlowConfig extends DataFlow::Configuration { - PropertiesFlowConfig() { this = "PropertiesFlowConfig" } +private module PropertiesFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof Properties } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { propertiesInput(sink, _) or propertiesStore(sink, _) } } + +private module PropertiesFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll index 8bd14b63ea6..0fdf69eb8a3 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll @@ -21,9 +21,7 @@ class CleartextStorageAdditionalTaintStep extends Unit { class SensitiveSource extends Expr instanceof SensitiveExpr { /** Holds if this source flows to the `sink`. */ predicate flowsTo(Expr sink) { - exists(SensitiveSourceFlowConfig conf | - conf.hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) - ) + SensitiveSourceFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) } } @@ -40,27 +38,25 @@ abstract class Storable extends Call { abstract Expr getAStore(); } -private class SensitiveSourceFlowConfig extends TaintTracking2::Configuration { - SensitiveSourceFlowConfig() { this = "SensitiveSourceFlowConfig" } +private module SensitiveSourceFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SensitiveExpr } + predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink } - override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextStorageSink } + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof CleartextStorageSanitizer } - override predicate isSanitizer(DataFlow::Node sanitizer) { - sanitizer instanceof CleartextStorageSanitizer - } - - override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) { + predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) { any(CleartextStorageAdditionalTaintStep c).step(n1, n2) } } +private module SensitiveSourceFlow = TaintTracking::Global; + private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer { DefaultCleartextStorageSanitizer() { this.getType() instanceof NumericType or this.getType() instanceof BooleanType or - exists(EncryptedValueFlowConfig conf | conf.hasFlow(_, this)) + EncryptedValueFlow::flowTo(this) } } @@ -76,12 +72,10 @@ private class EncryptedSensitiveMethodAccess extends MethodAccess { } /** Flow configuration for encryption methods flowing to inputs of persistent storage. */ -private class EncryptedValueFlowConfig extends DataFlow4::Configuration { - EncryptedValueFlowConfig() { this = "EncryptedValueFlowConfig" } +private module EncryptedValueFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof EncryptedSensitiveMethodAccess } - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof EncryptedSensitiveMethodAccess - } - - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SensitiveExpr } } + +private module EncryptedValueFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll index e67a2d1aa21..07764a251f7 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageSharedPrefsQuery.qll @@ -28,17 +28,17 @@ class SharedPreferencesEditorMethodAccess extends Storable, MethodAccess { /** Gets an input, for example `password` in `editor.putString("password", password);`. */ override Expr getAnInput() { - exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor | + exists(DataFlow::Node editor | sharedPreferencesInput(editor, result) and - conf.hasFlow(DataFlow::exprNode(this), editor) + SharedPreferencesFlow::flow(DataFlow::exprNode(this), editor) ) } /** Gets a store, for example `editor.commit();`. */ override Expr getAStore() { - exists(SharedPreferencesFlowConfig conf, DataFlow::Node editor | + exists(DataFlow::Node editor | sharedPreferencesStore(editor, result) and - conf.hasFlow(DataFlow::exprNode(this), editor) + SharedPreferencesFlow::flow(DataFlow::exprNode(this), editor) ) } } @@ -65,15 +65,15 @@ private predicate sharedPreferencesStore(DataFlow::Node editor, MethodAccess m) } /** Flow from `SharedPreferences.Editor` to either a setter or a store method. */ -private class SharedPreferencesFlowConfig extends DataFlow::Configuration { - SharedPreferencesFlowConfig() { this = "SharedPreferencesFlowConfig" } - - override predicate isSource(DataFlow::Node src) { +private module SharedPreferencesFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SharedPreferencesEditorMethodAccess } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sharedPreferencesInput(sink, _) or sharedPreferencesStore(sink, _) } } + +private module SharedPreferencesFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/ConditionalBypassQuery.qll b/java/ql/lib/semmle/code/java/security/ConditionalBypassQuery.qll index 9bff32eac02..a45afda4105 100644 --- a/java/ql/lib/semmle/code/java/security/ConditionalBypassQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ConditionalBypassQuery.qll @@ -37,9 +37,11 @@ private predicate endsWithStep(DataFlow::Node node1, DataFlow::Node node2) { } /** + * DEPRECATED: Use `ConditionalBypassFlow` instead. + * * A taint tracking configuration for untrusted data flowing to sensitive conditions. */ -class ConditionalBypassFlowConfig extends TaintTracking::Configuration { +deprecated class ConditionalBypassFlowConfig extends TaintTracking::Configuration { ConditionalBypassFlowConfig() { this = "ConditionalBypassFlowConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } @@ -50,3 +52,21 @@ class ConditionalBypassFlowConfig extends TaintTracking::Configuration { endsWithStep(node1, node2) } } + +/** + * A taint tracking configuration for untrusted data flowing to sensitive conditions. + */ +module ConditionalBypassFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { conditionControlsMethod(_, sink.asExpr()) } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + endsWithStep(node1, node2) + } +} + +/** + * Taint tracking flow for untrusted data flowing to sensitive conditions. + */ +module ConditionalBypassFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/ExternalAPIs.qll b/java/ql/lib/semmle/code/java/security/ExternalAPIs.qll index ef23d28a076..89b24006475 100644 --- a/java/ql/lib/semmle/code/java/security/ExternalAPIs.qll +++ b/java/ql/lib/semmle/code/java/security/ExternalAPIs.qll @@ -98,8 +98,12 @@ class ExternalApiDataNode extends DataFlow::Node { /** DEPRECATED: Alias for ExternalApiDataNode */ deprecated class ExternalAPIDataNode = ExternalApiDataNode; -/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */ -class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration { +/** + * DEPRECATED: Use `UntrustedDataToExternalApiFlow` instead. + * + * A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. + */ +deprecated class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration { UntrustedDataToExternalApiConfig() { this = "UntrustedDataToExternalAPIConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } @@ -107,17 +111,29 @@ class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } } +/** + * Taint tracking configuration for flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. + */ +module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } +} + +/** + * Tracks flow from untrusted data to external APIs. + */ +module UntrustedDataToExternalApiFlow = TaintTracking::Global; + /** DEPRECATED: Alias for UntrustedDataToExternalApiConfig */ deprecated class UntrustedDataToExternalAPIConfig = UntrustedDataToExternalApiConfig; /** A node representing untrusted data being passed to an external API. */ class UntrustedExternalApiDataNode extends ExternalApiDataNode { - UntrustedExternalApiDataNode() { any(UntrustedDataToExternalApiConfig c).hasFlow(_, this) } + UntrustedExternalApiDataNode() { UntrustedDataToExternalApiFlow::flowTo(this) } /** Gets a source of untrusted data which is passed to this external API data node. */ - DataFlow::Node getAnUntrustedSource() { - any(UntrustedDataToExternalApiConfig c).hasFlow(result, this) - } + DataFlow::Node getAnUntrustedSource() { UntrustedDataToExternalApiFlow::flow(result, this) } } /** DEPRECATED: Alias for UntrustedExternalApiDataNode */ diff --git a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll index 94b1877a4a3..6164a6663a0 100644 --- a/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/FragmentInjectionQuery.qll @@ -23,7 +23,11 @@ deprecated class FragmentInjectionTaintConf extends TaintTracking::Configuration } } -private module FragmentInjectionTaintConfig implements DataFlow::ConfigSig { +/** + * A taint-tracking configuration for unsafe user input + * that is used to create Android fragments dynamically. + */ +module FragmentInjectionTaintConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { sink instanceof FragmentInjectionSink } diff --git a/java/ql/lib/semmle/code/java/security/HardcodedCredentialsApiCallQuery.qll b/java/ql/lib/semmle/code/java/security/HardcodedCredentialsApiCallQuery.qll index fb786710d66..6080cd188b3 100644 --- a/java/ql/lib/semmle/code/java/security/HardcodedCredentialsApiCallQuery.qll +++ b/java/ql/lib/semmle/code/java/security/HardcodedCredentialsApiCallQuery.qll @@ -7,9 +7,11 @@ import semmle.code.java.dataflow.DataFlow import HardcodedCredentials /** + * DEPRECATED: Use `HardcodedCredentialApiCallFlow` instead. + * * A data-flow configuration that tracks flow from a hard-coded credential in a call to a sensitive Java API which may compromise security. */ -class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration { +deprecated class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration { HardcodedCredentialApiCallConfiguration() { this = "HardcodedCredentialApiCallConfiguration" } override predicate isSource(DataFlow::Node n) { @@ -52,3 +54,53 @@ class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration { n.asExpr().(MethodAccess).getMethod() instanceof MethodSystemGetenv } } + +/** + * A data-flow configuration that tracks flow from a hard-coded credential in a call to a sensitive Java API which may compromise security. + */ +module HardcodedCredentialApiCallConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { + n.asExpr() instanceof HardcodedExpr and + not n.asExpr().getEnclosingCallable() instanceof ToStringMethod + } + + predicate isSink(DataFlow::Node n) { n.asExpr() instanceof CredentialsApiSink } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + node1.asExpr().getType() instanceof TypeString and + ( + exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) | + node2.asExpr() = ma and + ma.getQualifier() = node1.asExpr() + ) + or + // These base64 routines are usually taint propagators, and this is not a general + // TaintTracking::Configuration, so we must specifically include them here + // as a common transform applied to a constant before passing to a remote API. + exists(MethodAccess ma | + ma.getMethod() + .hasQualifiedName([ + "java.util", "cn.hutool.core.codec", "org.apache.shiro.codec", + "apache.commons.codec.binary", "org.springframework.util" + ], ["Base64$Encoder", "Base64$Decoder", "Base64", "Base64Utils"], + [ + "encode", "encodeToString", "decode", "decodeBase64", "encodeBase64", + "encodeBase64Chunked", "encodeBase64String", "encodeBase64URLSafe", + "encodeBase64URLSafeString" + ]) + | + node1.asExpr() = ma.getArgument(0) and + node2.asExpr() = ma + ) + ) + } + + predicate isBarrier(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod() instanceof MethodSystemGetenv + } +} + +/** + * Tracks flow from a hard-coded credential in a call to a sensitive Java API which may compromise security. + */ +module HardcodedCredentialApiCallFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll b/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll index 50c76cefbca..cc827d34d46 100644 --- a/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/HttpsUrlsQuery.qll @@ -6,9 +6,11 @@ import semmle.code.java.frameworks.Networking import semmle.code.java.security.HttpsUrls /** + * DEPRECATED: Use `HttpsStringToUrlOpenMethodFlow` instead. + * * A taint tracking configuration for HTTP connections. */ -class HttpStringToUrlOpenMethodFlowConfig extends TaintTracking::Configuration { +deprecated class HttpStringToUrlOpenMethodFlowConfig extends TaintTracking::Configuration { HttpStringToUrlOpenMethodFlowConfig() { this = "HttpStringToUrlOpenMethodFlowConfig" } override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpStringLiteral } @@ -23,3 +25,25 @@ class HttpStringToUrlOpenMethodFlowConfig extends TaintTracking::Configuration { node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType } } + +/** + * A taint tracking configuration for HTTP connections. + */ +module HttpStringToUrlOpenMethodFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof HttpStringLiteral } + + predicate isSink(DataFlow::Node sink) { sink instanceof UrlOpenSink } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(HttpUrlsAdditionalTaintStep c).step(node1, node2) + } + + predicate isBarrier(DataFlow::Node node) { + node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType + } +} + +/** + * Detect taint flow of HTTP connections. + */ +module HttpStringToUrlOpenMethodFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll index 814dc93f8d0..c02abb4de81 100644 --- a/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll +++ b/java/ql/lib/semmle/code/java/security/ImplicitPendingIntentsQuery.qll @@ -7,10 +7,12 @@ import semmle.code.java.frameworks.android.PendingIntent import semmle.code.java.security.ImplicitPendingIntents /** + * DEPRECATED: Use `ImplicitPendingIntentStartFlow` instead. + * * A taint tracking configuration for implicit `PendingIntent`s * being wrapped in another implicit `Intent` that gets started. */ -class ImplicitPendingIntentStartConf extends TaintTracking::Configuration { +deprecated class ImplicitPendingIntentStartConf extends TaintTracking::Configuration { ImplicitPendingIntentStartConf() { this = "ImplicitPendingIntentStartConf" } override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { @@ -52,3 +54,50 @@ class ImplicitPendingIntentStartConf extends TaintTracking::Configuration { c instanceof DataFlow::ArrayContent } } + +/** + * A taint tracking configuration for implicit `PendingIntent`s + * being wrapped in another implicit `Intent` that gets started. + */ +module ImplicitPendingIntentStartConfig implements DataFlow::StateConfigSig { + class FlowState = DataFlow::FlowState; + + predicate isSource(DataFlow::Node source, FlowState state) { + source.(ImplicitPendingIntentSource).hasState(state) + } + + predicate isSink(DataFlow::Node sink, FlowState state) { + sink.(ImplicitPendingIntentSink).hasState(state) + } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof ExplicitIntentSanitizer } + + predicate isBarrier(DataFlow::Node node, FlowState state) { none() } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, node2) + } + + predicate isAdditionalFlowStep( + DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2 + ) { + any(ImplicitPendingIntentAdditionalTaintStep c).step(node1, state1, node2, state2) + } + + predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { + isSink(node, _) and + allowIntentExtrasImplicitRead(node, c) + or + isAdditionalFlowStep(node, _) and + c.(DataFlow::FieldContent).getType() instanceof PendingIntent + or + // Allow implicit reads of Intent arrays for steps like getActivities + // or sinks like startActivities + (isSink(node, _) or isAdditionalFlowStep(node, _, _, _)) and + node.getType().(Array).getElementType() instanceof TypeIntent and + c instanceof DataFlow::ArrayContent + } +} + +module ImplicitPendingIntentStartFlow = + TaintTracking::GlobalWithState; diff --git a/java/ql/lib/semmle/code/java/security/InsecureBasicAuthQuery.qll b/java/ql/lib/semmle/code/java/security/InsecureBasicAuthQuery.qll index 941aaafe580..60e16662d9a 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureBasicAuthQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureBasicAuthQuery.qll @@ -6,10 +6,12 @@ import semmle.code.java.security.InsecureBasicAuth import semmle.code.java.dataflow.TaintTracking /** + * DEPRECATED: Use `InsecureBasicAuthFlow` instead. + * * A taint tracking configuration for the Basic authentication scheme * being used in HTTP connections. */ -class BasicAuthFlowConfig extends TaintTracking::Configuration { +deprecated class BasicAuthFlowConfig extends TaintTracking::Configuration { BasicAuthFlowConfig() { this = "InsecureBasicAuth::BasicAuthFlowConfig" } override predicate isSource(DataFlow::Node src) { src instanceof InsecureBasicAuthSource } @@ -20,3 +22,22 @@ class BasicAuthFlowConfig extends TaintTracking::Configuration { any(HttpUrlsAdditionalTaintStep c).step(node1, node2) } } + +/** + * A taint tracking configuration for the Basic authentication scheme + * being used in HTTP connections. + */ +module BasicAuthFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof InsecureBasicAuthSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof InsecureBasicAuthSink } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(HttpUrlsAdditionalTaintStep c).step(node1, node2) + } +} + +/** + * Tracks flow for the Basic authentication scheme being used in HTTP connections. + */ +module InsecureBasicAuthFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll b/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll index 22ab9bbcbc9..a7514ceff96 100644 --- a/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll @@ -5,10 +5,12 @@ import semmle.code.java.dataflow.FlowSources import semmle.code.java.security.InsecureTrustManager /** + * DEPRECATED: Use `InsecureTrustManagerFlow` instead. + * * A configuration to model the flow of an insecure `TrustManager` * to the initialization of an SSL context. */ -class InsecureTrustManagerConfiguration extends DataFlow::Configuration { +deprecated class InsecureTrustManagerConfiguration extends DataFlow::Configuration { InsecureTrustManagerConfiguration() { this = "InsecureTrustManagerConfiguration" } override predicate isSource(DataFlow::Node source) { @@ -23,3 +25,21 @@ class InsecureTrustManagerConfiguration extends DataFlow::Configuration { c instanceof DataFlow::ArrayContent } } + +/** + * A configuration to model the flow of an insecure `TrustManager` + * to the initialization of an SSL context. + */ +module InsecureTrustManagerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof InsecureTrustManagerSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof InsecureTrustManagerSink } + + predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) { + (isSink(node) or isAdditionalFlowStep(node, _)) and + node.getType() instanceof Array and + c instanceof DataFlow::ArrayContent + } +} + +module InsecureTrustManagerFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index eb416d9647b..d0d51ffbf08 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -3,8 +3,12 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.InsufficientKeySize -/** A data flow configuration for tracking key sizes used in cryptographic algorithms. */ -class KeySizeConfiguration extends DataFlow::Configuration { +/** + * DEPRECATED: Use `KeySizeFlow` instead. + * + * A data flow configuration for tracking key sizes used in cryptographic algorithms. + */ +deprecated class KeySizeConfiguration extends DataFlow::Configuration { KeySizeConfiguration() { this = "KeySizeConfiguration" } override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { @@ -15,3 +19,30 @@ class KeySizeConfiguration extends DataFlow::Configuration { sink.(InsufficientKeySizeSink).hasState(state) } } + +/** + * A data flow configuration for tracking key sizes used in cryptographic algorithms. + */ +module KeySizeConfig implements DataFlow::StateConfigSig { + class FlowState = DataFlow::FlowState; + + predicate isSource(DataFlow::Node source, DataFlow::FlowState state) { + source.(InsufficientKeySizeSource).hasState(state) + } + + predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) { + sink.(InsufficientKeySizeSink).hasState(state) + } + + predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) { none() } + + predicate isAdditionalFlowStep( + DataFlow::Node node1, DataFlow::FlowState state1, DataFlow::Node node2, + DataFlow::FlowState state2 + ) { + none() + } +} + +/** Tracks key sizes used in cryptographic algorithms. */ +module KeySizeFlow = DataFlow::GlobalWithState; diff --git a/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulationQuery.qll b/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulationQuery.qll index 970cb4867fd..f563b4bf093 100644 --- a/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/IntentUriPermissionManipulationQuery.qll @@ -35,7 +35,10 @@ deprecated class IntentUriPermissionManipulationConf extends TaintTracking::Conf } } -private module IntentUriPermissionManipulationConfig implements DataFlow::ConfigSig { +/** + * A taint tracking configuration for user-provided Intents being returned to third party apps. + */ +module IntentUriPermissionManipulationConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { sink instanceof IntentUriPermissionManipulationSink } diff --git a/java/ql/lib/semmle/code/java/security/LogInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/LogInjectionQuery.qll index a26e08d3edc..2610870b383 100644 --- a/java/ql/lib/semmle/code/java/security/LogInjectionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/LogInjectionQuery.qll @@ -23,7 +23,10 @@ deprecated class LogInjectionConfiguration extends TaintTracking::Configuration } } -private module LogInjectionConfig implements DataFlow::ConfigSig { +/** + * A taint-tracking configuration for tracking untrusted user input used in log entries. + */ +module LogInjectionConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { sink instanceof LogInjectionSink } diff --git a/java/ql/lib/semmle/code/java/security/MissingJWTSignatureCheckQuery.qll b/java/ql/lib/semmle/code/java/security/MissingJWTSignatureCheckQuery.qll index 818a4c664fe..c316da2f965 100644 --- a/java/ql/lib/semmle/code/java/security/MissingJWTSignatureCheckQuery.qll +++ b/java/ql/lib/semmle/code/java/security/MissingJWTSignatureCheckQuery.qll @@ -5,10 +5,12 @@ import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.JWT /** + * DEPRECATED: Use `MissingJwtSignatureCheckFlow` instead. + * * Models flow from signing keys assignments to qualifiers of JWT insecure parsers. * This is used to determine whether a `JwtParser` performing unsafe parsing has a signing key set. */ -class MissingJwtSignatureCheckConf extends DataFlow::Configuration { +deprecated class MissingJwtSignatureCheckConf extends DataFlow::Configuration { MissingJwtSignatureCheckConf() { this = "SigningToExprDataFlow" } override predicate isSource(DataFlow::Node source) { @@ -21,3 +23,19 @@ class MissingJwtSignatureCheckConf extends DataFlow::Configuration { any(JwtParserWithInsecureParseAdditionalFlowStep c).step(node1, node2) } } + +/** + * Models flow from signing keys assignments to qualifiers of JWT insecure parsers. + * This is used to determine whether a `JwtParser` performing unsafe parsing has a signing key set. + */ +module MissingJwtSignatureCheckConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof JwtParserWithInsecureParseSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof JwtParserWithInsecureParseSink } + + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + any(JwtParserWithInsecureParseAdditionalFlowStep c).step(node1, node2) + } +} + +module MissingJwtSignatureCheckFlow = DataFlow::Global; diff --git a/java/ql/lib/semmle/code/java/security/PartialPathTraversalQuery.qll b/java/ql/lib/semmle/code/java/security/PartialPathTraversalQuery.qll index a43f90ae10d..1fd25df25aa 100644 --- a/java/ql/lib/semmle/code/java/security/PartialPathTraversalQuery.qll +++ b/java/ql/lib/semmle/code/java/security/PartialPathTraversalQuery.qll @@ -7,11 +7,13 @@ import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources /** + * DEPRECATED: Use `PartialPathTraversalFromRemoteFlow` instead. + * * A taint-tracking configuration for unsafe user input * that is used to validate against path traversal, but is insufficient * and remains vulnerable to Partial Path Traversal. */ -class PartialPathTraversalFromRemoteConfig extends TaintTracking::Configuration { +deprecated class PartialPathTraversalFromRemoteConfig extends TaintTracking::Configuration { PartialPathTraversalFromRemoteConfig() { this = "PartialPathTraversalFromRemoteConfig" } override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource } @@ -20,3 +22,20 @@ class PartialPathTraversalFromRemoteConfig extends TaintTracking::Configuration any(PartialPathTraversalMethodAccess ma).getQualifier() = node.asExpr() } } + +/** + * A taint-tracking configuration for unsafe user input + * that is used to validate against path traversal, but is insufficient + * and remains vulnerable to Partial Path Traversal. + */ +module PartialPathTraversalFromRemoteConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node node) { + any(PartialPathTraversalMethodAccess ma).getQualifier() = node.asExpr() + } +} + +/** Tracks flow of unsafe user input that is used to validate against path traversal, but is insufficient and remains vulnerable to Partial Path Traversal. */ +module PartialPathTraversalFromRemoteFlow = + TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/RandomQuery.qll b/java/ql/lib/semmle/code/java/security/RandomQuery.qll index 1674cefdb70..b14191f4dd4 100644 --- a/java/ql/lib/semmle/code/java/security/RandomQuery.qll +++ b/java/ql/lib/semmle/code/java/security/RandomQuery.qll @@ -2,7 +2,7 @@ import java import semmle.code.java.dataflow.DefUse -import semmle.code.java.dataflow.DataFlow6 +import semmle.code.java.dataflow.DataFlow import RandomDataSource /** @@ -29,20 +29,18 @@ private predicate isSeeded(RValue use) { ) } -private class PredictableSeedFlowConfiguration extends DataFlow6::Configuration { - PredictableSeedFlowConfiguration() { this = "Random::PredictableSeedFlowConfiguration" } +private module PredictableSeedFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof PredictableSeedExpr } - override predicate isSource(DataFlow6::Node source) { - source.asExpr() instanceof PredictableSeedExpr - } + predicate isSink(DataFlow::Node sink) { isSeeding(sink.asExpr(), _) } - override predicate isSink(DataFlow6::Node sink) { isSeeding(sink.asExpr(), _) } - - override predicate isAdditionalFlowStep(DataFlow6::Node node1, DataFlow6::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { predictableCalcStep(node1.asExpr(), node2.asExpr()) } } +private module PredictableSeedFlow = DataFlow::Global; + private predicate predictableCalcStep(Expr e1, Expr e2) { e2.(BinaryExpr).hasOperands(e1, any(PredictableSeedExpr p)) or @@ -81,7 +79,7 @@ private predicate predictableCalcStep(Expr e1, Expr e2) { private predicate safelySeeded(RValue use) { exists(Expr arg | isSeeding(arg, use) and - not exists(PredictableSeedFlowConfiguration conf | conf.hasFlowToExpr(arg)) + not PredictableSeedFlow::flowToExpr(arg) ) or exists(GetRandomData da, RValue seeduse | @@ -118,9 +116,7 @@ private predicate isSeeding(Expr arg, RValue use) { private predicate isSeedingSource(Expr arg, RValue use, Expr source) { isSeeding(arg, use) and - exists(PredictableSeedFlowConfiguration conf | - conf.hasFlow(DataFlow6::exprNode(source), DataFlow6::exprNode(arg)) - ) + PredictableSeedFlow::flow(DataFlow::exprNode(source), DataFlow::exprNode(arg)) } private predicate isRandomSeeding(MethodAccess m, Expr arg) { diff --git a/java/ql/lib/semmle/code/java/security/RsaWithoutOaepQuery.qll b/java/ql/lib/semmle/code/java/security/RsaWithoutOaepQuery.qll index 0d9df09bb74..848a1c2b990 100644 --- a/java/ql/lib/semmle/code/java/security/RsaWithoutOaepQuery.qll +++ b/java/ql/lib/semmle/code/java/security/RsaWithoutOaepQuery.qll @@ -26,7 +26,10 @@ deprecated class RsaWithoutOaepConfig extends DataFlow::Configuration { } } -private module RsaWithoutOaepConfig implements DataFlow::ConfigSig { +/** + * A configuration for finding RSA ciphers initialized without using OAEP padding. + */ +module RsaWithoutOaepConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { exists(CompileTimeConstantExpr specExpr, string spec | specExpr.getStringValue() = spec and diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index ea687d32a0a..d9ed2b970b0 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -49,7 +49,7 @@ deprecated class SensitiveLoggerConfiguration extends TaintTracking::Configurati } /** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */ -private module SensitiveLoggerConfig implements DataFlow::ConfigSig { +module SensitiveLoggerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr() instanceof CredentialExpr } predicate isSink(DataFlow::Node sink) { sinkNode(sink, "logging") } diff --git a/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll b/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll index 64dee88b7fd..38d426adc5a 100644 --- a/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll +++ b/java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll @@ -83,25 +83,23 @@ private class ArrayUpdate extends Expr { /** * A config that tracks dataflow from creating an array to an operation that updates it. */ -private class ArrayUpdateConfig extends DataFlow2::Configuration { - ArrayUpdateConfig() { this = "ArrayUpdateConfig" } +private module ArrayUpdateConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source.asExpr() instanceof StaticByteArrayCreation } - override predicate isSource(DataFlow::Node source) { - source.asExpr() instanceof StaticByteArrayCreation - } + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(ArrayUpdate upd).getArray() } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(ArrayUpdate upd).getArray() } - - override predicate isBarrierOut(DataFlow::Node node) { this.isSink(node) } + predicate isBarrierOut(DataFlow::Node node) { isSink(node) } } +private module ArrayUpdateFlow = DataFlow::Global; + /** * A source that defines an array that doesn't get updated. */ private class StaticInitializationVectorSource extends DataFlow::Node { StaticInitializationVectorSource() { exists(StaticByteArrayCreation array | array = this.asExpr() | - not exists(ArrayUpdateConfig config | config.hasFlow(DataFlow2::exprNode(array), _)) and + not ArrayUpdateFlow::flow(DataFlow2::exprNode(array), _) and // Reduce FPs from utility methods that return an empty array in an exceptional case not exists(ReturnStmt ret | array.getADimension().(CompileTimeConstantExpr).getIntValue() = 0 and @@ -146,9 +144,11 @@ private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, Da } /** + * DEPRECATED: Use `StaticInitializationVectorFlow` instead. + * * A config that tracks dataflow to initializing a cipher with a static initialization vector. */ -class StaticInitializationVectorConfig extends TaintTracking::Configuration { +deprecated class StaticInitializationVectorConfig extends TaintTracking::Configuration { StaticInitializationVectorConfig() { this = "StaticInitializationVectorConfig" } override predicate isSource(DataFlow::Node source) { @@ -161,3 +161,19 @@ class StaticInitializationVectorConfig extends TaintTracking::Configuration { createInitializationVectorSpecStep(fromNode, toNode) } } + +/** + * A config that tracks dataflow to initializing a cipher with a static initialization vector. + */ +module StaticInitializationVectorConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof StaticInitializationVectorSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink } + + predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + createInitializationVectorSpecStep(fromNode, toNode) + } +} + +/** Tracks the flow from a static initialization vector to the initialization of a cipher */ +module StaticInitializationVectorFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll index ee87fdc64ee..0b45e150c0c 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeCertTrustQuery.qll @@ -6,9 +6,11 @@ import semmle.code.java.security.UnsafeCertTrust import semmle.code.java.security.Encryption /** + * DEPRECATED: Use `SslEndpointIdentificationFlow` instead. + * * A taint flow configuration for SSL connections created without a proper certificate trust configuration. */ -class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { +deprecated class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { SslEndpointIdentificationFlowConfig() { this = "SslEndpointIdentificationFlowConfig" } override predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit } @@ -20,30 +22,44 @@ class SslEndpointIdentificationFlowConfig extends TaintTracking::Configuration { } } +/** + * A taint flow configuration for SSL connections created without a proper certificate trust configuration. + */ +module SslEndpointIdentificationFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof SslConnectionInit } + + predicate isSink(DataFlow::Node sink) { sink instanceof SslConnectionCreation } + + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof SslUnsafeCertTrustSanitizer } +} + +/** + * Taint flow for SSL connections created without a proper certificate trust configuration. + */ +module SslEndpointIdentificationFlow = TaintTracking::Global; + /** * An SSL object that was assigned a safe `SSLParameters` object and can be considered safe. */ private class SslConnectionWithSafeSslParameters extends SslUnsafeCertTrustSanitizer { SslConnectionWithSafeSslParameters() { - exists(SafeSslParametersFlowConfig config, DataFlow::Node safe, DataFlow::Node sanitizer | - config.hasFlowTo(safe) and + exists(DataFlow::Node safe, DataFlow::Node sanitizer | + SafeSslParametersFlow::flowTo(safe) and sanitizer = DataFlow::exprNode(safe.asExpr().(Argument).getCall().getQualifier()) and DataFlow::localFlow(sanitizer, this) ) } } -private class SafeSslParametersFlowConfig extends DataFlow2::Configuration { - SafeSslParametersFlowConfig() { this = "SafeSslParametersFlowConfig" } - - override predicate isSource(DataFlow::Node source) { +private module SafeSslParametersFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { exists(MethodAccess ma | ma instanceof SafeSetEndpointIdentificationAlgorithm and DataFlow::getInstanceArgument(ma) = source.(DataFlow::PostUpdateNode).getPreUpdateNode() ) } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma, RefType t | t instanceof SslSocket or t instanceof SslEngine | ma.getMethod().hasName("setSSLParameters") and ma.getMethod().getDeclaringType().getAnAncestor() = t and @@ -52,6 +68,8 @@ private class SafeSslParametersFlowConfig extends DataFlow2::Configuration { } } +private module SafeSslParametersFlow = DataFlow::Global; + /** * A call to `SSLParameters.setEndpointIdentificationAlgorithm` with a non-null and non-empty parameter. */ diff --git a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolutionQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolutionQuery.qll index b59c4b79655..424edace82a 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolutionQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeContentUriResolutionQuery.qll @@ -26,7 +26,10 @@ deprecated class UnsafeContentResolutionConf extends TaintTracking::Configuratio } } -private module UnsafeContentResolutionConfig implements DataFlow::ConfigSig { +/** + * A taint-tracking configuration to find paths from remote sources to content URI resolutions. + */ +module UnsafeContentResolutionConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { sink instanceof ContentUriResolutionSink } diff --git a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll index 6d0ff888d82..7e995e5cbaf 100644 --- a/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/UnsafeDeserializationQuery.qll @@ -35,15 +35,13 @@ private class XmlDecoderReadObjectMethod extends Method { } } -private class SafeXStream extends DataFlow2::Configuration { - SafeXStream() { this = "UnsafeDeserialization::SafeXStream" } - - override predicate isSource(DataFlow::Node src) { +private module SafeXStreamConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { any(XStreamEnableWhiteListing ma).getQualifier().(VarAccess).getVariable().getAnAccess() = src.asExpr() } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof XStreamReadObjectMethod @@ -51,26 +49,26 @@ private class SafeXStream extends DataFlow2::Configuration { } } -private class SafeKryo extends DataFlow2::Configuration { - SafeKryo() { this = "UnsafeDeserialization::SafeKryo" } +private module SafeXStreamFlow = DataFlow::Global; - override predicate isSource(DataFlow::Node src) { +private module SafeKryoConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { any(KryoEnableWhiteListing ma).getQualifier().(VarAccess).getVariable().getAnAccess() = src.asExpr() } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getQualifier() and ma.getMethod() instanceof KryoReadObjectMethod ) } - override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { - this.stepKryoPoolBuilderFactoryArgToConstructor(node1, node2) or - this.stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(node1, node2) or - this.stepKryoPoolBuilderChainMethod(node1, node2) or - this.stepKryoPoolBorrowMethod(node1, node2) + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + stepKryoPoolBuilderFactoryArgToConstructor(node1, node2) or + stepKryoPoolRunMethodAccessQualifierToFunctionalArgument(node1, node2) or + stepKryoPoolBuilderChainMethod(node1, node2) or + stepKryoPoolBorrowMethod(node1, node2) } /** @@ -126,6 +124,8 @@ private class SafeKryo extends DataFlow2::Configuration { } } +private module SafeKryoFlow = DataFlow::Global; + /** * Holds if `ma` is a call that deserializes data from `sink`. */ @@ -145,11 +145,11 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) { or m instanceof XStreamReadObjectMethod and sink = ma.getAnArgument() and - not exists(SafeXStream sxs | sxs.hasFlowToExpr(ma.getQualifier())) + not SafeXStreamFlow::flowToExpr(ma.getQualifier()) or m instanceof KryoReadObjectMethod and sink = ma.getAnArgument() and - not exists(SafeKryo sk | sk.hasFlowToExpr(ma.getQualifier())) + not SafeKryoFlow::flowToExpr(ma.getQualifier()) or m instanceof MethodApacheSerializationUtilsDeserialize and sink = ma.getArgument(0) @@ -181,17 +181,17 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) { ma.getMethod() instanceof ObjectMapperReadMethod and sink = ma.getArgument(0) and ( - exists(UnsafeTypeConfig config | config.hasFlowToExpr(ma.getAnArgument())) + UnsafeTypeFlow::flowToExpr(ma.getAnArgument()) or - exists(EnableJacksonDefaultTypingConfig config | config.hasFlowToExpr(ma.getQualifier())) + EnableJacksonDefaultTypingFlow::flowToExpr(ma.getQualifier()) or hasArgumentWithUnsafeJacksonAnnotation(ma) ) and - not exists(SafeObjectMapperConfig config | config.hasFlowToExpr(ma.getQualifier())) + not SafeObjectMapperFlow::flowToExpr(ma.getQualifier()) or m instanceof JabsorbUnmarshallMethod and sink = ma.getArgument(2) and - any(UnsafeTypeConfig config).hasFlowToExpr(ma.getArgument(1)) + UnsafeTypeFlow::flowToExpr(ma.getArgument(1)) or m instanceof JabsorbFromJsonMethod and sink = ma.getArgument(0) @@ -200,7 +200,7 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) { sink = ma.getArgument(0) and ( // User controls the target type for deserialization - any(UnsafeTypeConfig c).hasFlowToExpr(ma.getArgument(1)) + UnsafeTypeFlow::flowToExpr(ma.getArgument(1)) or // jodd.json.JsonParser may be configured for unrestricted deserialization to user-specified types joddJsonParserConfiguredUnsafely(ma.getQualifier()) @@ -211,7 +211,7 @@ predicate unsafeDeserialization(MethodAccess ma, Expr sink) { or m instanceof GsonDeserializeMethod and sink = ma.getArgument(0) and - any(UnsafeTypeConfig config).hasFlowToExpr(ma.getArgument(1)) + UnsafeTypeFlow::flowToExpr(ma.getArgument(1)) ) } @@ -223,10 +223,80 @@ class UnsafeDeserializationSink extends DataFlow::ExprNode { MethodAccess getMethodAccess() { unsafeDeserialization(result, this.getExpr()) } } +/** Holds if `node` is a sanitizer for unsafe deserialization */ +private predicate isUnsafeDeserializationSanitizer(DataFlow::Node node) { + exists(ClassInstanceExpr cie | + cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and + cie = node.asExpr() and + SafeJsonIoFlow::flowToExpr(cie.getArgument(1)) + ) + or + exists(MethodAccess ma | + ma.getMethod() instanceof JsonIoJsonToJavaMethod and + ma.getArgument(0) = node.asExpr() and + SafeJsonIoFlow::flowToExpr(ma.getArgument(1)) + ) + or + exists(MethodAccess ma | + // Sanitize the input to jodd.json.JsonParser.parse et al whenever it appears + // to be called with an explicit class argument limiting those types that can + // be instantiated during deserialization. + ma.getMethod() instanceof JoddJsonParseMethod and + ma.getArgument(1).getType() instanceof TypeClass and + not ma.getArgument(1) instanceof NullLiteral and + not ma.getArgument(1).getType().getName() = ["Class", "Class"] and + node.asExpr() = ma.getAnArgument() + ) + or + exists(MethodAccess ma | + // Sanitize the input to flexjson.JSONDeserializer.deserialize whenever it appears + // to be called with an explicit class argument limiting those types that can + // be instantiated during deserialization, or if the deserializer has already been + // configured to use a specified root class. + ma.getMethod() instanceof FlexjsonDeserializeMethod and + node.asExpr() = ma.getAnArgument() and + ( + ma.getArgument(1).getType() instanceof TypeClass and + not ma.getArgument(1) instanceof NullLiteral and + not ma.getArgument(1).getType().getName() = ["Class", "Class"] + or + isSafeFlexjsonDeserializer(ma.getQualifier()) + ) + ) +} + +/** Taint step for Unsafe deserialization */ +private predicate isUnsafeDeserializationTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(ClassInstanceExpr cie | + cie.getArgument(0) = pred.asExpr() and + cie = succ.asExpr() and + ( + cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or + cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or + cie.getConstructor().getDeclaringType().getAnAncestor() instanceof UnsafeHessianInput or + cie.getConstructor().getDeclaringType() instanceof BurlapInput + ) + ) + or + exists(MethodAccess ma | + ma.getMethod() instanceof BurlapInputInitMethod and + ma.getArgument(0) = pred.asExpr() and + ma.getQualifier() = succ.asExpr() + ) + or + createJacksonJsonParserStep(pred, succ) + or + createJacksonTreeNodeStep(pred, succ) + or + intentFlowsToParcel(pred, succ) +} + /** + * DEPRECATED: Use `UnsafeDeserializationFlow` instead. + * * Tracks flows from remote user input to a deserialization sink. */ -class UnsafeDeserializationConfig extends TaintTracking::Configuration { +deprecated class UnsafeDeserializationConfig extends TaintTracking::Configuration { UnsafeDeserializationConfig() { this = "UnsafeDeserializationConfig" } override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } @@ -234,72 +304,27 @@ class UnsafeDeserializationConfig extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink } override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { - exists(ClassInstanceExpr cie | - cie.getArgument(0) = pred.asExpr() and - cie = succ.asExpr() and - ( - cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader or - cie.getConstructor().getDeclaringType() instanceof YamlBeansReader or - cie.getConstructor().getDeclaringType().getAnAncestor() instanceof UnsafeHessianInput or - cie.getConstructor().getDeclaringType() instanceof BurlapInput - ) - ) - or - exists(MethodAccess ma | - ma.getMethod() instanceof BurlapInputInitMethod and - ma.getArgument(0) = pred.asExpr() and - ma.getQualifier() = succ.asExpr() - ) - or - createJacksonJsonParserStep(pred, succ) - or - createJacksonTreeNodeStep(pred, succ) - or - intentFlowsToParcel(pred, succ) + isUnsafeDeserializationTaintStep(pred, succ) } - override predicate isSanitizer(DataFlow::Node node) { - exists(ClassInstanceExpr cie | - cie.getConstructor().getDeclaringType() instanceof JsonIoJsonReader and - cie = node.asExpr() and - exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(cie.getArgument(1))) - ) - or - exists(MethodAccess ma | - ma.getMethod() instanceof JsonIoJsonToJavaMethod and - ma.getArgument(0) = node.asExpr() and - exists(SafeJsonIoConfig sji | sji.hasFlowToExpr(ma.getArgument(1))) - ) - or - exists(MethodAccess ma | - // Sanitize the input to jodd.json.JsonParser.parse et al whenever it appears - // to be called with an explicit class argument limiting those types that can - // be instantiated during deserialization. - ma.getMethod() instanceof JoddJsonParseMethod and - ma.getArgument(1).getType() instanceof TypeClass and - not ma.getArgument(1) instanceof NullLiteral and - not ma.getArgument(1).getType().getName() = ["Class", "Class"] and - node.asExpr() = ma.getAnArgument() - ) - or - exists(MethodAccess ma | - // Sanitize the input to flexjson.JSONDeserializer.deserialize whenever it appears - // to be called with an explicit class argument limiting those types that can - // be instantiated during deserialization, or if the deserializer has already been - // configured to use a specified root class. - ma.getMethod() instanceof FlexjsonDeserializeMethod and - node.asExpr() = ma.getAnArgument() and - ( - ma.getArgument(1).getType() instanceof TypeClass and - not ma.getArgument(1) instanceof NullLiteral and - not ma.getArgument(1).getType().getName() = ["Class", "Class"] - or - isSafeFlexjsonDeserializer(ma.getQualifier()) - ) - ) - } + override predicate isSanitizer(DataFlow::Node node) { isUnsafeDeserializationSanitizer(node) } } +/** Tracks flows from remote user input to a deserialization sink. */ +private module UnsafeDeserializationConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserializationSink } + + predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + isUnsafeDeserializationTaintStep(pred, succ) + } + + predicate isBarrier(DataFlow::Node node) { isUnsafeDeserializationSanitizer(node) } +} + +module UnsafeDeserializationFlow = TaintTracking::Global; + /** * Gets a safe usage of the `use` method of Flexjson, which could be: * use(String, ...) where the path is null or @@ -350,18 +375,9 @@ predicate looksLikeResolveClassStep(DataFlow::Node fromNode, DataFlow::Node toNo ) } -/** - * Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance) - * passed to a deserialization method. - * - * If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type. - */ -class UnsafeTypeConfig extends TaintTracking2::Configuration { - UnsafeTypeConfig() { this = "UnsafeTypeConfig" } - - override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { +/** A sink representing an argument of a deserialization method */ +private class UnsafeTypeSink extends DataFlow::Node { + UnsafeTypeSink() { exists(MethodAccess ma, int i, Expr arg | i > 0 and ma.getArgument(i) = arg | ( ma.getMethod() instanceof ObjectMapperReadMethod @@ -378,25 +394,75 @@ class UnsafeTypeConfig extends TaintTracking2::Configuration { or arg.getType().(RefType).hasQualifiedName("java.lang.reflect", "Type") ) and - arg = sink.asExpr() + arg = this.asExpr() ) } +} + +private predicate isUnsafeTypeAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + resolveClassStep(fromNode, toNode) or + looksLikeResolveClassStep(fromNode, toNode) or + intentFlowsToParcel(fromNode, toNode) +} + +/** + * DEPRECATED: Use `UnsafeTypeFlow` instead. + * + * Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance) + * passed to a deserialization method. + * + * If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type. + */ +deprecated class UnsafeTypeConfig extends TaintTracking2::Configuration { + UnsafeTypeConfig() { this = "UnsafeTypeConfig" } + + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeTypeSink } /** * Holds if `fromNode` to `toNode` is a dataflow step that resolves a class * or at least looks like resolving a class. */ override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { - resolveClassStep(fromNode, toNode) or - looksLikeResolveClassStep(fromNode, toNode) or - intentFlowsToParcel(fromNode, toNode) + isUnsafeTypeAdditionalTaintStep(fromNode, toNode) } } /** + * Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance) + * passed to a deserialization method. + * + * If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type. + */ +module UnsafeTypeConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeTypeSink } + + /** + * Holds if `fromNode` to `toNode` is a dataflow step that resolves a class + * or at least looks like resolving a class. + */ + predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + isUnsafeTypeAdditionalTaintStep(fromNode, toNode) + } +} + +/** + * Tracks flow from a remote source to a type descriptor (e.g. a `java.lang.Class` instance) + * passed to a deserialization method. + * + * If this is user-controlled, arbitrary code could be executed while instantiating the user-specified type. + */ +module UnsafeTypeFlow = TaintTracking::Global; + +/** + * DEPRECATED: Use `EnableJacksonDefaultTypingFlow` instead. + * * Tracks flow from `enableDefaultTyping` calls to a subsequent Jackson deserialization method call. */ -class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration { +deprecated class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration { EnableJacksonDefaultTypingConfig() { this = "EnableJacksonDefaultTypingConfig" } override predicate isSource(DataFlow::Node src) { @@ -406,13 +472,43 @@ class EnableJacksonDefaultTypingConfig extends DataFlow2::Configuration { override predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier } } +private module EnableJacksonDefaultTypingConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { + any(EnableJacksonDefaultTyping ma).getQualifier() = src.asExpr() + } + + predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier } +} + /** + * Tracks flow from `enableDefaultTyping` calls to a subsequent Jackson deserialization method call. + */ +module EnableJacksonDefaultTypingFlow = DataFlow::Global; + +/** Dataflow step that creates an `ObjectMapper` via a builder. */ +private predicate isObjectMapperBuilderAdditionalFlowStep( + DataFlow::Node fromNode, DataFlow::Node toNode +) { + exists(MethodAccess ma, Method m | m = ma.getMethod() | + m.getDeclaringType() instanceof MapperBuilder and + m.getReturnType() + .(RefType) + .hasQualifiedName("com.fasterxml.jackson.databind.json", + ["JsonMapper$Builder", "JsonMapper"]) and + fromNode.asExpr() = ma.getQualifier() and + ma = toNode.asExpr() + ) +} + +/** + * DEPRECATED: Use `SafeObjectMapperFlow` instead. + * * Tracks flow from calls that set a type validator to a subsequent Jackson deserialization method call, * including across builder method calls. * * Such a Jackson deserialization method call is safe because validation will likely prevent instantiating unexpected types. */ -class SafeObjectMapperConfig extends DataFlow2::Configuration { +deprecated class SafeObjectMapperConfig extends DataFlow2::Configuration { SafeObjectMapperConfig() { this = "SafeObjectMapperConfig" } override predicate isSource(DataFlow::Node src) { @@ -426,18 +522,38 @@ class SafeObjectMapperConfig extends DataFlow2::Configuration { * that configures or creates an `ObjectMapper` via a builder. */ override predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { - exists(MethodAccess ma, Method m | m = ma.getMethod() | - m.getDeclaringType() instanceof MapperBuilder and - m.getReturnType() - .(RefType) - .hasQualifiedName("com.fasterxml.jackson.databind.json", - ["JsonMapper$Builder", "JsonMapper"]) and - fromNode.asExpr() = ma.getQualifier() and - ma = toNode.asExpr() - ) + isObjectMapperBuilderAdditionalFlowStep(fromNode, toNode) } } +/** + * Tracks flow from calls that set a type validator to a subsequent Jackson deserialization method call, + * including across builder method calls. + * + * Such a Jackson deserialization method call is safe because validation will likely prevent instantiating unexpected types. + */ +module SafeObjectMapperConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof SetPolymorphicTypeValidatorSource } + + predicate isSink(DataFlow::Node sink) { sink instanceof ObjectMapperReadQualifier } + + /** + * Holds if `fromNode` to `toNode` is a dataflow step + * that configures or creates an `ObjectMapper` via a builder. + */ + predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + isObjectMapperBuilderAdditionalFlowStep(fromNode, toNode) + } +} + +/** + * Tracks flow from calls that set a type validator to a subsequent Jackson deserialization method call, + * including across builder method calls. + * + * Such a Jackson deserialization method call is safe because validation will likely prevent instantiating unexpected types. + */ +module SafeObjectMapperFlow = DataFlow::Global; + /** * A method that configures Jodd's JsonParser, either enabling dangerous deserialization to * arbitrary Java types or restricting the types that can be instantiated. @@ -454,20 +570,12 @@ private class JoddJsonParserConfigurationMethodQualifier extends DataFlow::ExprN } } -/** - * Configuration tracking flow from methods that configure `jodd.json.JsonParser`'s class - * instantiation feature to a `.parse` call on the same parser. - */ -private class JoddJsonParserConfigurationMethodConfig extends DataFlow2::Configuration { - JoddJsonParserConfigurationMethodConfig() { - this = "UnsafeDeserialization::JoddJsonParserConfigurationMethodConfig" - } - - override predicate isSource(DataFlow::Node src) { +private module JoddJsonParserConfigurationMethodConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src instanceof JoddJsonParserConfigurationMethodQualifier } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | ma.getMethod() instanceof JoddJsonParseMethod and sink.asExpr() = ma.getQualifier() // The class type argument @@ -475,6 +583,13 @@ private class JoddJsonParserConfigurationMethodConfig extends DataFlow2::Configu } } +/** + * Configuration tracking flow from methods that configure `jodd.json.JsonParser`'s class + * instantiation feature to a `.parse` call on the same parser. + */ +private module JoddJsonParserConfigurationMethodFlow = + DataFlow::Global; + /** * Gets the qualifier to a method call that configures a `jodd.json.JsonParser` instance unsafely. * @@ -512,10 +627,8 @@ private DataFlow::Node getASafelyConfiguredParser() { * and which never appears to be configured safely. */ private predicate joddJsonParserConfiguredUnsafely(Expr parserExpr) { - exists(DataFlow::Node parser, JoddJsonParserConfigurationMethodConfig config | - parser.asExpr() = parserExpr - | - config.hasFlow(getAnUnsafelyConfiguredParser(), parser) and - not config.hasFlow(getASafelyConfiguredParser(), parser) + exists(DataFlow::Node parser | parser.asExpr() = parserExpr | + JoddJsonParserConfigurationMethodFlow::flow(getAnUnsafelyConfiguredParser(), parser) and + not JoddJsonParserConfigurationMethodFlow::flow(getASafelyConfiguredParser(), parser) ) } diff --git a/java/ql/lib/semmle/code/java/security/XmlParsers.qll b/java/ql/lib/semmle/code/java/security/XmlParsers.qll index 8cdf962584d..dd28d8b0117 100644 --- a/java/ql/lib/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/lib/semmle/code/java/security/XmlParsers.qll @@ -81,24 +81,18 @@ class DocumentBuilderParse extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } override predicate isSafe() { - exists(SafeDocumentBuilderToDocumentBuilderParseFlowConfig conf | - conf.hasFlowToExpr(this.getQualifier()) - ) + SafeDocumentBuilderToDocumentBuilderParseFlow::flowToExpr(this.getQualifier()) } } -private class SafeDocumentBuilderToDocumentBuilderParseFlowConfig extends DataFlow2::Configuration { - SafeDocumentBuilderToDocumentBuilderParseFlowConfig() { - this = "XmlParsers::SafeDocumentBuilderToDocumentBuilderParseFlowConfig" - } +private module SafeDocumentBuilderToDocumentBuilderParseFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDocumentBuilder } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDocumentBuilder } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(DocumentBuilderParse dbp).getQualifier() } - override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { + predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { exists(RefType t, ReturnStmt ret, Method m | node2.asExpr().(ClassInstanceExpr).getConstructedType().getSourceDeclaration() = t and t.getASourceSupertype+().hasQualifiedName("java.lang", "ThreadLocal") and @@ -117,9 +111,12 @@ private class SafeDocumentBuilderToDocumentBuilderParseFlowConfig extends DataFl ) } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeDocumentBuilderToDocumentBuilderParseFlow = + DataFlow::Global; + /** * A `ParserConfig` specific to `DocumentBuilderFactory`. */ @@ -198,31 +195,27 @@ private class DocumentBuilderConstruction extends MethodAccess { } } -private class SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlowConfig extends DataFlow3::Configuration +private module SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlowConfig implements + DataFlow::ConfigSig { - SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlowConfig() { - this = "XmlParsers::SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlowConfig" - } + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDocumentBuilderFactory } - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof SafeDocumentBuilderFactory - } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(DocumentBuilderConstruction dbc).getQualifier() } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlow = + DataFlow::Global; + /** * A `DocumentBuilder` created from a safely configured `DocumentBuilderFactory`. */ class SafeDocumentBuilder extends DocumentBuilderConstruction { SafeDocumentBuilder() { - exists(SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlowConfig conf | - conf.hasFlowToExpr(this.getQualifier()) - ) + SafeDocumentBuilderFactoryToDocumentBuilderConstructionFlow::flowToExpr(this.getQualifier()) } } @@ -252,27 +245,24 @@ class XmlInputFactoryStreamReader extends XmlParserCall { } override predicate isSafe() { - exists(SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig conf | - conf.hasFlowToExpr(this.getQualifier()) - ) + SafeXmlInputFactoryToXmlInputFactoryReaderFlow::flowToExpr(this.getQualifier()) } } -private class SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig extends DataFlow2::Configuration { - SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig() { - this = "XmlParsers::SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig" - } +private module SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeXmlInputFactory } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeXmlInputFactory } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(XmlInputFactoryStreamReader xifsr).getQualifier() or sink.asExpr() = any(XmlInputFactoryEventReader xifer).getQualifier() } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeXmlInputFactoryToXmlInputFactoryReaderFlow = + DataFlow::Global; + /** A call to `XMLInputFactory.createEventReader`. */ class XmlInputFactoryEventReader extends XmlParserCall { XmlInputFactoryEventReader() { @@ -290,9 +280,7 @@ class XmlInputFactoryEventReader extends XmlParserCall { } override predicate isSafe() { - exists(SafeXmlInputFactoryToXmlInputFactoryReaderFlowConfig conf | - conf.hasFlowToExpr(this.getQualifier()) - ) + SafeXmlInputFactoryToXmlInputFactoryReaderFlow::flowToExpr(this.getQualifier()) } } @@ -387,27 +375,24 @@ class SaxBuilderParse extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } override predicate isSafe() { - exists(SafeSaxBuilderToSaxBuilderParseFlowConfig conf | conf.hasFlowToExpr(this.getQualifier())) + SafeSaxBuilderToSaxBuilderParseFlow::flowToExpr(this.getQualifier()) } } /** DEPRECATED: Alias for SaxBuilderParse */ deprecated class SAXBuilderParse = SaxBuilderParse; -private class SafeSaxBuilderToSaxBuilderParseFlowConfig extends DataFlow2::Configuration { - SafeSaxBuilderToSaxBuilderParseFlowConfig() { - this = "XmlParsers::SafeSAXBuilderToSAXBuilderParseFlowConfig" - } +private module SafeSaxBuilderToSaxBuilderParseFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxBuilder } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxBuilder } + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(SaxBuilderParse sax).getQualifier() } - override predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(SaxBuilderParse sax).getQualifier() - } - - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeSaxBuilderToSaxBuilderParseFlow = + DataFlow::Global; + /** * A `ParserConfig` specific to `SAXBuilder`. */ @@ -478,9 +463,7 @@ class SaxParserParse extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } - override predicate isSafe() { - exists(SafeSaxParserFlowConfig sp | sp.hasFlowToExpr(this.getQualifier())) - } + override predicate isSafe() { SafeSaxParserFlow::flowToExpr(this.getQualifier()) } } /** DEPRECATED: Alias for SaxParserParse */ @@ -536,14 +519,10 @@ class SafeSaxParserFactory extends VarAccess { /** DEPRECATED: Alias for SafeSaxParserFactory */ deprecated class SafeSAXParserFactory = SafeSaxParserFactory; -private class SafeSaxParserFactoryToNewSaxParserFlowConfig extends DataFlow5::Configuration { - SafeSaxParserFactoryToNewSaxParserFlowConfig() { - this = "XmlParsers::SafeSAXParserFactoryToNewSAXParserFlowConfig" - } +private module SafeSaxParserFactoryToNewSaxParserFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxParserFactory } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxParserFactory } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma, Method m | sink.asExpr() = ma.getQualifier() and ma.getMethod() = m and @@ -552,31 +531,32 @@ private class SafeSaxParserFactoryToNewSaxParserFlowConfig extends DataFlow5::Co ) } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } -private class SafeSaxParserFlowConfig extends DataFlow4::Configuration { - SafeSaxParserFlowConfig() { this = "XmlParsers::SafeSAXParserFlowConfig" } +private module SafeSaxParserFactoryToNewSaxParserFlow = + DataFlow::Global; - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxParser } +private module SafeSaxParserFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxParser } - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof SaxParser ) } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeSaxParserFlow = DataFlow::Global; + /** A `SaxParser` created from a safely configured `SaxParserFactory`. */ class SafeSaxParser extends MethodAccess { SafeSaxParser() { - exists(SafeSaxParserFactoryToNewSaxParserFlowConfig sdf | - this.getMethod().getDeclaringType() instanceof SaxParserFactory and - this.getMethod().hasName("newSAXParser") and - sdf.hasFlowToExpr(this.getQualifier()) - ) + this.getMethod().getDeclaringType() instanceof SaxParserFactory and + this.getMethod().hasName("newSAXParser") and + SafeSaxParserFactoryToNewSaxParserFlow::flowToExpr(this.getQualifier()) } } @@ -606,9 +586,7 @@ class SaxReaderRead extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } - override predicate isSafe() { - exists(SafeSaxReaderFlowConfig sr | sr.hasFlowToExpr(this.getQualifier())) - } + override predicate isSafe() { SafeSaxReaderFlow::flowToExpr(this.getQualifier()) } } /** DEPRECATED: Alias for SaxReaderRead */ @@ -628,20 +606,20 @@ class SaxReaderConfig extends ParserConfig { /** DEPRECATED: Alias for SaxReaderConfig */ deprecated class SAXReaderConfig = SaxReaderConfig; -private class SafeSaxReaderFlowConfig extends DataFlow4::Configuration { - SafeSaxReaderFlowConfig() { this = "XmlParsers::SafeSAXReaderFlowConfig" } +private module SafeSaxReaderFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxReader } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxReader } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma | sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof SaxReader ) } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeSaxReaderFlow = DataFlow::Global; + /** A safely configured `SaxReader`. */ class SafeSaxReader extends VarAccess { SafeSaxReader() { @@ -715,18 +693,16 @@ class XmlReaderConfig extends ParserConfig { /** DEPRECATED: Alias for XmlReaderConfig */ deprecated class XMLReaderConfig = XmlReaderConfig; -private class ExplicitlySafeXmlReaderFlowConfig extends DataFlow3::Configuration { - ExplicitlySafeXmlReaderFlowConfig() { this = "XmlParsers::ExplicitlySafeXMLReaderFlowConfig" } +private module ExplicitlySafeXmlReaderFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof ExplicitlySafeXmlReader } - override predicate isSource(DataFlow::Node src) { - src.asExpr() instanceof ExplicitlySafeXmlReader - } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SafeXmlReaderFlowSink } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SafeXmlReaderFlowSink } - - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module ExplicitlySafeXmlReaderFlow = DataFlow::Global; + /** An argument to a safe XML reader. */ class SafeXmlReaderFlowSink extends Expr { SafeXmlReaderFlowSink() { @@ -774,40 +750,35 @@ class ExplicitlySafeXmlReader extends VarAccess { /** Holds if `SafeXmlReaderFlowSink` detects flow from this to `sink` */ predicate flowsTo(SafeXmlReaderFlowSink sink) { - any(ExplicitlySafeXmlReaderFlowConfig conf) - .hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) + ExplicitlySafeXmlReaderFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) } } /** DEPRECATED: Alias for ExplicitlySafeXmlReader */ deprecated class ExplicitlySafeXMLReader = ExplicitlySafeXmlReader; -private class CreatedSafeXmlReaderFlowConfig extends DataFlow3::Configuration { - CreatedSafeXmlReaderFlowConfig() { this = "XmlParsers::CreatedSafeXMLReaderFlowConfig" } +private module CreatedSafeXmlReaderFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof CreatedSafeXmlReader } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof CreatedSafeXmlReader } + predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SafeXmlReaderFlowSink } - override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof SafeXmlReaderFlowSink } - - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module CreatedSafeXmlReaderFlow = DataFlow::Global; + /** An `XmlReader` that is obtained from a safe source. */ class CreatedSafeXmlReader extends Call { CreatedSafeXmlReader() { //Obtained from SAXParser - exists(SafeSaxParserFlowConfig safeParser | - this.(MethodAccess).getMethod().getDeclaringType() instanceof SaxParser and - this.(MethodAccess).getMethod().hasName("getXMLReader") and - safeParser.hasFlowToExpr(this.getQualifier()) - ) + this.(MethodAccess).getMethod().getDeclaringType() instanceof SaxParser and + this.(MethodAccess).getMethod().hasName("getXMLReader") and + SafeSaxParserFlow::flowToExpr(this.getQualifier()) or //Obtained from SAXReader - exists(SafeSaxReaderFlowConfig safeReader | - this.(MethodAccess).getMethod().getDeclaringType() instanceof SaxReader and - this.(MethodAccess).getMethod().hasName("getXMLReader") and - safeReader.hasFlowToExpr(this.getQualifier()) - ) + this.(MethodAccess).getMethod().getDeclaringType() instanceof SaxReader and + this.(MethodAccess).getMethod().hasName("getXMLReader") and + SafeSaxReaderFlow::flowToExpr(this.getQualifier()) or exists(RefType secureReader, string package | this.(ClassInstanceExpr).getConstructedType() = secureReader and @@ -818,8 +789,7 @@ class CreatedSafeXmlReader extends Call { /** Holds if `CreatedSafeXmlReaderFlowConfig` detects flow from this to `sink` */ predicate flowsTo(SafeXmlReaderFlowSink sink) { - any(CreatedSafeXmlReaderFlowConfig conf) - .hasFlow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) + CreatedSafeXmlReaderFlow::flow(DataFlow::exprNode(this), DataFlow::exprNode(sink)) } } @@ -975,26 +945,23 @@ class TransformerTransform extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } override predicate isSafe() { - exists(SafeTransformerToTransformerTransformFlowConfig st | - st.hasFlowToExpr(this.getQualifier()) - ) + SafeTransformerToTransformerTransformFlow::flowToExpr(this.getQualifier()) } } -private class SafeTransformerToTransformerTransformFlowConfig extends DataFlow2::Configuration { - SafeTransformerToTransformerTransformFlowConfig() { - this = "XmlParsers::SafeTransformerToTransformerTransformFlowConfig" - } +private module SafeTransformerToTransformerTransformFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformer } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformer } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(TransformerTransform tt).getQualifier() } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeTransformerToTransformerTransformFlow = + DataFlow::Global; + /** A call to `Transformer.newTransformer` with source. */ class TransformerFactorySource extends XmlParserCall { TransformerFactorySource() { @@ -1007,9 +974,7 @@ class TransformerFactorySource extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } - override predicate isSafe() { - exists(SafeTransformerFactoryFlowConfig stf | stf.hasFlowToExpr(this.getQualifier())) - } + override predicate isSafe() { SafeTransformerFactoryFlow::flowToExpr(this.getQualifier()) } } /** A `ParserConfig` specific to `TransformerFactory`. */ @@ -1024,10 +989,12 @@ class TransformerFactoryConfig extends TransformerConfig { } /** + * DEPRECATED: Use `SafeTransformerFactoryFlow` instead. + * * A dataflow configuration that identifies `TransformerFactory` and `SAXTransformerFactory` * instances that have been safely configured. */ -class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration { +deprecated class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration { SafeTransformerFactoryFlowConfig() { this = "XmlParsers::SafeTransformerFactoryFlowConfig" } override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformerFactory } @@ -1042,6 +1009,29 @@ class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration { override int fieldFlowBranchLimit() { result = 0 } } +/** + * A dataflow configuration that identifies `TransformerFactory` and `SAXTransformerFactory` + * instances that have been safely configured. + */ +module SafeTransformerFactoryFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformerFactory } + + predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + sink.asExpr() = ma.getQualifier() and + ma.getMethod().getDeclaringType() instanceof TransformerFactory + ) + } + + int fieldFlowBranchLimit() { result = 0 } +} + +/** + * Identifies `TransformerFactory` and `SAXTransformerFactory` + * instances that have been safely configured. + */ +module SafeTransformerFactoryFlow = DataFlow::Global; + /** A safely configured `TransformerFactory`. */ class SafeTransformerFactory extends VarAccess { SafeTransformerFactory() { @@ -1059,11 +1049,11 @@ class SafeTransformerFactory extends VarAccess { /** A `Transformer` created from a safely configured `TransformerFactory`. */ class SafeTransformer extends MethodAccess { SafeTransformer() { - exists(SafeTransformerFactoryFlowConfig stf, Method m | + exists(Method m | this.getMethod() = m and m.getDeclaringType() instanceof TransformerFactory and m.hasName("newTransformer") and - stf.hasFlowToExpr(this.getQualifier()) + SafeTransformerFactoryFlow::flowToExpr(this.getQualifier()) ) } } @@ -1085,9 +1075,7 @@ class SaxTransformerFactoryNewXmlFilter extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } - override predicate isSafe() { - exists(SafeTransformerFactoryFlowConfig stf | stf.hasFlowToExpr(this.getQualifier())) - } + override predicate isSafe() { SafeTransformerFactoryFlow::flowToExpr(this.getQualifier()) } } /** DEPRECATED: Alias for SaxTransformerFactoryNewXmlFilter */ @@ -1123,26 +1111,23 @@ class SchemaFactoryNewSchema extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } override predicate isSafe() { - exists(SafeSchemaFactoryToSchemaFactoryNewSchemaFlowConfig ssf | - ssf.hasFlowToExpr(this.getQualifier()) - ) + SafeSchemaFactoryToSchemaFactoryNewSchemaFlow::flowToExpr(this.getQualifier()) } } -private class SafeSchemaFactoryToSchemaFactoryNewSchemaFlowConfig extends DataFlow2::Configuration { - SafeSchemaFactoryToSchemaFactoryNewSchemaFlowConfig() { - this = "XmlParsers::SafeSchemaFactoryToSchemaFactoryNewSchemaFlowConfig" - } +private module SafeSchemaFactoryToSchemaFactoryNewSchemaFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSchemaFactory } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSchemaFactory } - - override predicate isSink(DataFlow::Node sink) { + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(SchemaFactoryNewSchema sfns).getQualifier() } - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } +private module SafeSchemaFactoryToSchemaFactoryNewSchemaFlow = + DataFlow::Global; + /** A safely configured `SchemaFactory`. */ class SafeSchemaFactory extends VarAccess { SafeSchemaFactory() { diff --git a/java/ql/lib/semmle/code/java/security/XxeQuery.qll b/java/ql/lib/semmle/code/java/security/XxeQuery.qll index e1df8a1e601..18f7dd9e357 100644 --- a/java/ql/lib/semmle/code/java/security/XxeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/XxeQuery.qll @@ -1,7 +1,7 @@ /** Provides default definitions to be used in XXE queries. */ import java -private import semmle.code.java.dataflow.TaintTracking2 +private import semmle.code.java.dataflow.TaintTracking private import semmle.code.java.security.XmlParsers import semmle.code.java.security.Xxe @@ -11,7 +11,7 @@ import semmle.code.java.security.Xxe */ private class DefaultXxeSink extends XxeSink { DefaultXxeSink() { - not exists(SafeSaxSourceFlowConfig safeSource | safeSource.hasFlowTo(this)) and + not SafeSaxSourceFlow::flowTo(this) and exists(XmlParserCall parse | parse.getSink() = this.asExpr() and not parse.isSafe() @@ -22,14 +22,12 @@ private class DefaultXxeSink extends XxeSink { /** * A taint-tracking configuration for safe XML readers used to parse XML documents. */ -private class SafeSaxSourceFlowConfig extends TaintTracking2::Configuration { - SafeSaxSourceFlowConfig() { this = "SafeSaxSourceFlowConfig" } +private module SafeSaxSourceFlowConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource } - override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource } + predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(XmlParserCall parse).getSink() } - override predicate isSink(DataFlow::Node sink) { - sink.asExpr() = any(XmlParserCall parse).getSink() - } - - override int fieldFlowBranchLimit() { result = 0 } + int fieldFlowBranchLimit() { result = 0 } } + +private module SafeSaxSourceFlow = TaintTracking::Global; diff --git a/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll b/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll index e85e130e381..4d7f963e968 100644 --- a/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll +++ b/java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll @@ -65,7 +65,7 @@ deprecated predicate hasPolynomialReDoSResult( } /** A configuration for Polynomial ReDoS queries. */ -private module PolynomialRedosConfig implements DataFlow::ConfigSig { +module PolynomialRedosConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { diff --git a/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql b/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql index b4a2e209513..fdbb34b2247 100644 --- a/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql +++ b/java/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql @@ -13,10 +13,10 @@ import java import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.ExternalAPIs -import DataFlow::PathGraph +import UntrustedDataToExternalApiFlow::PathGraph -from UntrustedDataToExternalApiConfig config, DataFlow::PathNode source, DataFlow::PathNode sink -where config.hasFlowPath(source, sink) +from UntrustedDataToExternalApiFlow::PathNode source, UntrustedDataToExternalApiFlow::PathNode sink +where UntrustedDataToExternalApiFlow::flowPath(source, sink) select sink, source, sink, "Call to " + sink.getNode().(ExternalApiDataNode).getMethodDescription() + " with untrusted data from $@.", source, source.toString() diff --git a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalFromRemote.ql b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalFromRemote.ql index 8ef4ab45853..ef62f433ae6 100644 --- a/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalFromRemote.ql +++ b/java/ql/src/Security/CWE/CWE-023/PartialPathTraversalFromRemote.ql @@ -11,10 +11,12 @@ */ import semmle.code.java.security.PartialPathTraversalQuery -import DataFlow::PathGraph +import PartialPathTraversalFromRemoteFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink -where any(PartialPathTraversalFromRemoteConfig config).hasFlowPath(source, sink) +from + PartialPathTraversalFromRemoteFlow::PathNode source, + PartialPathTraversalFromRemoteFlow::PathNode sink +where PartialPathTraversalFromRemoteFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Partial Path Traversal Vulnerability due to insufficient guard against path traversal from $@.", source, "user-supplied data" diff --git a/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql b/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql index 669c4e6f946..258e0f87112 100644 --- a/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql +++ b/java/ql/src/Security/CWE/CWE-1204/StaticInitializationVector.ql @@ -13,9 +13,9 @@ import java import semmle.code.java.security.StaticInitializationVectorQuery -import DataFlow::PathGraph +import StaticInitializationVectorFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, StaticInitializationVectorConfig conf -where conf.hasFlowPath(source, sink) +from StaticInitializationVectorFlow::PathNode source, StaticInitializationVectorFlow::PathNode sink +where StaticInitializationVectorFlow::flowPath(source, sink) select sink.getNode(), source, sink, "A $@ should not be used for encryption.", source.getNode(), "static initialization vector" diff --git a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql index 2925d588c74..eb5241ac87a 100644 --- a/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql +++ b/java/ql/src/Security/CWE/CWE-273/UnsafeCertTrust.ql @@ -18,7 +18,5 @@ import semmle.code.java.security.UnsafeCertTrustQuery from Expr unsafeTrust where unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet or - exists(SslEndpointIdentificationFlowConfig config | - config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) - ) + SslEndpointIdentificationFlow::flowTo(DataFlow::exprNode(unsafeTrust)) select unsafeTrust, "Unsafe configuration of trusted certificates." diff --git a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql index 755f2b5a4a7..4904c08b195 100644 --- a/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql +++ b/java/ql/src/Security/CWE/CWE-295/InsecureTrustManager.ql @@ -13,10 +13,10 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.InsecureTrustManagerQuery -import DataFlow::PathGraph +import InsecureTrustManagerFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink -where any(InsecureTrustManagerConfiguration cfg).hasFlowPath(source, sink) +from InsecureTrustManagerFlow::PathNode source, InsecureTrustManagerFlow::PathNode sink +where InsecureTrustManagerFlow::flowPath(source, sink) select sink, source, sink, "This uses $@, which is defined in $@ and trusts any certificate.", source, "TrustManager", source.getNode().asExpr().(ClassInstanceExpr).getConstructedType() as type, type.nestedName() diff --git a/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql b/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql index 745a2d6dfad..4bcf99fd4b6 100644 --- a/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql +++ b/java/ql/src/Security/CWE/CWE-319/HttpsUrls.ql @@ -12,9 +12,9 @@ import java import semmle.code.java.security.HttpsUrlsQuery -import DataFlow::PathGraph +import HttpStringToUrlOpenMethodFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink -where any(HttpStringToUrlOpenMethodFlowConfig c).hasFlowPath(source, sink) +from HttpStringToUrlOpenMethodFlow::PathNode source, HttpStringToUrlOpenMethodFlow::PathNode sink +where HttpStringToUrlOpenMethodFlow::flowPath(source, sink) select sink.getNode(), source, sink, "URL may have been constructed with HTTP protocol, using $@.", source.getNode(), "this HTTP URL" diff --git a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql index 2cf3d9115b3..39e4c3e64e5 100644 --- a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql +++ b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql @@ -13,10 +13,10 @@ import java import semmle.code.java.security.InsufficientKeySizeQuery -import DataFlow::PathGraph +import KeySizeFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, KeySizeConfiguration cfg -where cfg.hasFlowPath(source, sink) +from KeySizeFlow::PathNode source, KeySizeFlow::PathNode sink +where KeySizeFlow::flowPath(source, sink) select sink.getNode(), source, sink, "This $@ is less than the recommended key size of " + source.getState() + " bits.", source.getNode(), "key size" diff --git a/java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql b/java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql index 648321ec3ab..077d7a67370 100644 --- a/java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql +++ b/java/ql/src/Security/CWE/CWE-347/MissingJWTSignatureCheck.ql @@ -12,9 +12,9 @@ import java import semmle.code.java.security.MissingJWTSignatureCheckQuery -import DataFlow::PathGraph +import MissingJwtSignatureCheckFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, MissingJwtSignatureCheckConf conf -where conf.hasFlowPath(source, sink) +from MissingJwtSignatureCheckFlow::PathNode source, MissingJwtSignatureCheckFlow::PathNode sink +where MissingJwtSignatureCheckFlow::flowPath(source, sink) select sink.getNode(), source, sink, "This parses a $@, but the signature is not verified.", source.getNode(), "JWT signing key" diff --git a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql index d494d92e657..1c5660653a6 100644 --- a/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql +++ b/java/ql/src/Security/CWE/CWE-502/UnsafeDeserialization.ql @@ -13,9 +13,9 @@ import java import semmle.code.java.security.UnsafeDeserializationQuery -import DataFlow::PathGraph +import UnsafeDeserializationFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, UnsafeDeserializationConfig conf -where conf.hasFlowPath(source, sink) +from UnsafeDeserializationFlow::PathNode source, UnsafeDeserializationFlow::PathNode sink +where UnsafeDeserializationFlow::flowPath(source, sink) select sink.getNode().(UnsafeDeserializationSink).getMethodAccess(), source, sink, "Unsafe deserialization depends on a $@.", source.getNode(), "user-provided value" diff --git a/java/ql/src/Security/CWE/CWE-522/InsecureBasicAuth.ql b/java/ql/src/Security/CWE/CWE-522/InsecureBasicAuth.ql index 069245b4a3e..ae74b355e5f 100644 --- a/java/ql/src/Security/CWE/CWE-522/InsecureBasicAuth.ql +++ b/java/ql/src/Security/CWE/CWE-522/InsecureBasicAuth.ql @@ -16,9 +16,9 @@ import java import semmle.code.java.security.InsecureBasicAuthQuery -import DataFlow::PathGraph +import InsecureBasicAuthFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, BasicAuthFlowConfig config -where config.hasFlowPath(source, sink) +from InsecureBasicAuthFlow::PathNode source, InsecureBasicAuthFlow::PathNode sink +where InsecureBasicAuthFlow::flowPath(source, sink) select sink.getNode(), source, sink, "Insecure basic authentication from a $@.", source.getNode(), "HTTP URL" diff --git a/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql b/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql index 2b84f4b8065..410cea0ed03 100644 --- a/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql +++ b/java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql @@ -11,10 +11,9 @@ */ import semmle.code.java.security.HardcodedCredentialsApiCallQuery -import DataFlow::PathGraph +import HardcodedCredentialApiCallFlow::PathGraph -from - DataFlow::PathNode source, DataFlow::PathNode sink, HardcodedCredentialApiCallConfiguration conf -where conf.hasFlowPath(source, sink) +from HardcodedCredentialApiCallFlow::PathNode source, HardcodedCredentialApiCallFlow::PathNode sink +where HardcodedCredentialApiCallFlow::flowPath(source, sink) select source.getNode(), source, sink, "Hard-coded value flows to $@.", sink.getNode(), "sensitive API call" diff --git a/java/ql/src/Security/CWE/CWE-807/ConditionalBypass.ql b/java/ql/src/Security/CWE/CWE-807/ConditionalBypass.ql index 3cfa2079685..d012ae33810 100644 --- a/java/ql/src/Security/CWE/CWE-807/ConditionalBypass.ql +++ b/java/ql/src/Security/CWE/CWE-807/ConditionalBypass.ql @@ -15,15 +15,15 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.ConditionalBypassQuery -import DataFlow::PathGraph +import ConditionalBypassFlow::PathGraph from - DataFlow::PathNode source, DataFlow::PathNode sink, MethodAccess m, Expr e, - ConditionalBypassFlowConfig conf + ConditionalBypassFlow::PathNode source, ConditionalBypassFlow::PathNode sink, MethodAccess m, + Expr e where conditionControlsMethod(m, e) and sink.getNode().asExpr() = e and - conf.hasFlowPath(source, sink) + ConditionalBypassFlow::flowPath(source, sink) select m, source, sink, "Sensitive method may not be executed depending on a $@, which flows from $@.", e, "this condition", source.getNode(), "user-controlled value" diff --git a/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql b/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql index 58eb6300868..f4065ceeae6 100644 --- a/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql +++ b/java/ql/src/Security/CWE/CWE-927/ImplicitPendingIntents.ql @@ -15,10 +15,10 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.security.ImplicitPendingIntentsQuery -import DataFlow::PathGraph +import ImplicitPendingIntentStartFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink -where any(ImplicitPendingIntentStartConf conf).hasFlowPath(source, sink) +from ImplicitPendingIntentStartFlow::PathNode source, ImplicitPendingIntentStartFlow::PathNode sink +where ImplicitPendingIntentStartFlow::flowPath(source, sink) select sink.getNode(), source, sink, "$@ and sent to an unspecified third party through a PendingIntent.", source.getNode(), "An implicit Intent is created" diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll b/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll index 3f44faa54d0..61008ae63aa 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll @@ -232,9 +232,7 @@ class SaxTransformerFactoryNewTransformerHandler extends XmlParserCall { override Expr getSink() { result = this.getArgument(0) } - override predicate isSafe() { - exists(SafeTransformerFactoryFlowConfig stf | stf.hasFlowToExpr(this.getQualifier())) - } + override predicate isSafe() { SafeTransformerFactoryFlow::flowToExpr(this.getQualifier()) } } /** DEPRECATED: Alias for SaxTransformerFactoryNewTransformerHandler */ diff --git a/java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalFromRemoteTest.ql b/java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalFromRemoteTest.ql index 7f441e62d4e..a07eb6892c3 100644 --- a/java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalFromRemoteTest.ql +++ b/java/ql/test/query-tests/security/CWE-023/semmle/tests/PartialPathTraversalFromRemoteTest.ql @@ -1,21 +1,24 @@ import java -import TestUtilities.InlineFlowTest +import TestUtilities.InlineExpectationsTest import semmle.code.java.security.PartialPathTraversalQuery -class EnableLegacy extends EnableLegacyConfiguration { - EnableLegacy() { exists(this) } -} - class TestRemoteSource extends RemoteFlowSource { TestRemoteSource() { this.asParameter().hasName(["dir", "path"]) } override string getSourceType() { result = "TestSource" } } -class Test extends InlineFlowTest { - override DataFlow::Configuration getValueFlowConfig() { none() } +class Test extends InlineExpectationsTest { + Test() { this = "PartialPathTraversalFromRemoteTest" } - override TaintTracking::Configuration getTaintFlowConfig() { - result instanceof PartialPathTraversalFromRemoteConfig + override string getARelevantTag() { result = "hasTaintFlow" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasTaintFlow" and + exists(DataFlow::Node sink | PartialPathTraversalFromRemoteFlow::flowTo(sink) | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) } } diff --git a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql index 653a3cefa7a..ceaf8e11a4f 100644 --- a/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql +++ b/java/ql/test/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql @@ -9,7 +9,7 @@ class StaticInitializationVectorTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "staticInitializationVector" and - exists(DataFlow::Node sink, StaticInitializationVectorConfig conf | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | StaticInitializationVectorFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql index 344faa7f86b..fd18ccc27eb 100644 --- a/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql +++ b/java/ql/test/query-tests/security/CWE-273/UnsafeCertTrustTest.ql @@ -12,9 +12,7 @@ class UnsafeCertTrustTest extends InlineExpectationsTest { exists(Expr unsafeTrust | unsafeTrust instanceof RabbitMQEnableHostnameVerificationNotSet or - exists(SslEndpointIdentificationFlowConfig config | - config.hasFlowTo(DataFlow::exprNode(unsafeTrust)) - ) + SslEndpointIdentificationFlow::flowTo(DataFlow::exprNode(unsafeTrust)) | unsafeTrust.getLocation() = location and element = unsafeTrust.toString() and diff --git a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql index fb00ff33b34..0f068d04679 100644 --- a/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql +++ b/java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql @@ -1,13 +1,18 @@ import java import semmle.code.java.security.InsecureTrustManagerQuery -import TestUtilities.InlineFlowTest +import TestUtilities.InlineExpectationsTest -class EnableLegacy extends EnableLegacyConfiguration { - EnableLegacy() { exists(this) } -} +class InsecureTrustManagerTest extends InlineExpectationsTest { + InsecureTrustManagerTest() { this = "InsecureTrustManagerTest" } -class InsecureTrustManagerTest extends InlineFlowTest { - override DataFlow::Configuration getValueFlowConfig() { - result = any(InsecureTrustManagerConfiguration c) + override string getARelevantTag() { result = "hasValueFlow" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasValueFlow" and + exists(DataFlow::Node sink | InsecureTrustManagerFlow::flowTo(sink) | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) } } diff --git a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql index 12f667a67f3..1384a36e4ce 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.ql @@ -9,7 +9,7 @@ class InsufficientKeySizeTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasInsufficientKeySize" and - exists(DataFlow::PathNode sink | exists(KeySizeConfiguration cfg | cfg.hasFlowPath(_, sink)) | + exists(KeySizeFlow::PathNode sink | KeySizeFlow::flowPath(_, sink) | sink.getNode().getLocation() = location and element = sink.getNode().toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-347/MissingJWTSignatureCheckTest.ql b/java/ql/test/query-tests/security/CWE-347/MissingJWTSignatureCheckTest.ql index b4f4c1c445e..df6867bbefe 100644 --- a/java/ql/test/query-tests/security/CWE-347/MissingJWTSignatureCheckTest.ql +++ b/java/ql/test/query-tests/security/CWE-347/MissingJWTSignatureCheckTest.ql @@ -9,7 +9,7 @@ class HasMissingJwtSignatureCheckTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasMissingJwtSignatureCheck" and - exists(DataFlow::Node sink, MissingJwtSignatureCheckConf conf | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | MissingJwtSignatureCheckFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-502/UnsafeDeserialization.ql b/java/ql/test/query-tests/security/CWE-502/UnsafeDeserialization.ql index a2ba654a540..4d10c798e33 100644 --- a/java/ql/test/query-tests/security/CWE-502/UnsafeDeserialization.ql +++ b/java/ql/test/query-tests/security/CWE-502/UnsafeDeserialization.ql @@ -9,7 +9,7 @@ class UnsafeDeserializationTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "unsafeDeserialization" and - exists(DataFlow::Node sink, UnsafeDeserializationConfig conf | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | UnsafeDeserializationFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-522/InsecureBasicAuthTest.ql b/java/ql/test/query-tests/security/CWE-522/InsecureBasicAuthTest.ql index b593c5476ef..29057edce71 100644 --- a/java/ql/test/query-tests/security/CWE-522/InsecureBasicAuthTest.ql +++ b/java/ql/test/query-tests/security/CWE-522/InsecureBasicAuthTest.ql @@ -9,7 +9,7 @@ class HasInsecureBasicAuthTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasInsecureBasicAuth" and - exists(DataFlow::Node sink, BasicAuthFlowConfig conf | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | InsecureBasicAuthFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.ql b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.ql index 79ce1738f38..7d7b8e2d2a5 100644 --- a/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.ql +++ b/java/ql/test/query-tests/security/CWE-798/semmle/tests/HardcodedCredentialsApiCall.ql @@ -9,9 +9,7 @@ class HardcodedCredentialsApiCallTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "HardcodedCredentialsApiCall" and - exists(DataFlow::Node sink, HardcodedCredentialApiCallConfiguration conf | - conf.hasFlow(_, sink) - | + exists(DataFlow::Node sink | HardcodedCredentialApiCallFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.ql b/java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.ql index ef676d4a8f0..20ca408f1b2 100644 --- a/java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.ql +++ b/java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.ql @@ -9,7 +9,7 @@ class ConditionalBypassTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasConditionalBypassTest" and - exists(DataFlow::Node sink, ConditionalBypassFlowConfig conf | conf.hasFlowTo(sink) | + exists(DataFlow::Node sink | ConditionalBypassFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/query-tests/security/CWE-927/ImplicitPendingIntentsTest.ql b/java/ql/test/query-tests/security/CWE-927/ImplicitPendingIntentsTest.ql index 871dcd2cef2..972653380aa 100644 --- a/java/ql/test/query-tests/security/CWE-927/ImplicitPendingIntentsTest.ql +++ b/java/ql/test/query-tests/security/CWE-927/ImplicitPendingIntentsTest.ql @@ -9,7 +9,7 @@ class ImplicitPendingIntentsTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasImplicitPendingIntent" and - exists(DataFlow::Node sink | any(ImplicitPendingIntentStartConf c).hasFlowTo(sink) | + exists(DataFlow::Node sink | ImplicitPendingIntentStartFlow::flowTo(sink) | sink.getLocation() = location and element = sink.toString() and value = ""