diff --git a/java/ql/integration-tests/all-platforms/kotlin/default-parameter-mad-flow/test.ext.yml b/java/ql/integration-tests/all-platforms/kotlin/default-parameter-mad-flow/test.ext.yml index 1632b5a8080..0ffc8f55394 100644 --- a/java/ql/integration-tests/all-platforms/kotlin/default-parameter-mad-flow/test.ext.yml +++ b/java/ql/integration-tests/all-platforms/kotlin/default-parameter-mad-flow/test.ext.yml @@ -26,3 +26,9 @@ extensions: - ["", "LibKt", True, "extensionSink", "(String,int,int)", "", "Argument[1]", "kotlinMadFlowTest", "manual"] - ["", "SinkClass", True, "memberSink", "(int,int)", "", "Argument[0]", "kotlinMadFlowTest", "manual"] - ["", "SinkClass", True, "extensionMemberSink", "(String,int,int)", "", "Argument[1]", "kotlinMadFlowTest", "manual"] + + - addsTo: + pack: codeql/java-all + extensible: supportedThreatModel + data: + - ["kotlinMadFlowTest"] diff --git a/java/ql/lib/ext/threat-grouping.model.yml b/java/ql/lib/ext/threat-grouping.model.yml new file mode 100644 index 00000000000..54e3e3a6cf2 --- /dev/null +++ b/java/ql/lib/ext/threat-grouping.model.yml @@ -0,0 +1,56 @@ +extensions: + + # An incomplete set of sources database reads + - addsTo: + pack: codeql/java-all + extensible: sinkModel + data: + - ["java.io", "OutputStream", True, "write", "(byte[])", "", "Argument[0]", "sql", "manual"] + - ["java.io", "OutputStream", True, "write", "(byte[],int,int)", "", "Argument[0]", "sql", "manual"] + - ["java.io", "OutputStream", True, "write", "(int)", "", "Argument[0]", "sql", "manual"] + + + - addsTo: + pack: codeql/java-all + extensible: sourceModel + data: + # Create a few + - ["java.sql", "PreparedStatement", True, "executeQuery", "()", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "PreparedStatement", True, "getMetaData", "()", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "PreparedStatement", True, "getParameterMetaData", "", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "Statement", True, "executeQuery", "(String)", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "Statement", True, "getResultSet", "()", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "Statement", True, "getGeneratedKeys", "()", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "Statement", True, "getConnection", "()", "", "ReturnValue", "sql", "manual"] + - ["java.sql", "ResultSet", True, "getInt", "(int)", "", "Argument[0]", "sql", "manual"] + + # This one is not defined elsewhere. Why not? I can't get my example working without it. + - ["java.io", "InputStream", True, "read", "()", "", "ReturnValue", "remote", "manual"] + + + # Create a graph of parent-child relationships between threat models and their kinds + # The left side is a kind of threat model. The right side groups the kinds together. + - addsTo: + pack: codeql/java-all + extensible: threatModelGrouping + data: + - ["android-widget", "android"] + - ["android-external-storage-dir", "android"] + - ["contentprovider", "android"] + - ["request", "remote"] + - ["response", "remote"] + - ["sql", "local"] + - ["remote", "standard"] + + # Not sure if these should really go in the standard threat model, but we need them for tests to pass + - ["android-external-storage-dir", "standard"] + - ["contentprovider", "standard"] + - ["android-widget", "standard"] + - ["android-web-resource-response", "standard"] + - ["uri-path", "standard"] + + # Provide a default, empty supportedThreatModel + - addsTo: + pack: codeql/java-all + extensible: supportedThreatModel + data: [] diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index 65f87ccb68f..83c7a040eed 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -84,6 +84,7 @@ private import internal.FlowSummaryImpl::Private::External private import internal.FlowSummaryImplSpecific as FlowSummaryImplSpecific private import internal.AccessPathSyntax private import ExternalFlowExtensions as Extensions +private import ExternalFlowConfiguration as ConfiguredExtensions private import FlowSummary /** @@ -135,10 +136,12 @@ predicate sourceModel( string package, string type, boolean subtypes, string name, string signature, string ext, string output, string kind, string provenance ) { - Extensions::sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance) - or - any(ActiveExperimentalModels q) - .sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance) + ConfiguredExtensions::supportedSourceModel(kind) and ( + Extensions::sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance) + or + any(ActiveExperimentalModels q) + .sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance) + ) } /** Holds if a sink model exists for the given parameters. */ @@ -282,12 +285,13 @@ module ModelValidation { not kind.matches("qltest%") and result = "Invalid kind \"" + kind + "\" in sink model." ) - or - exists(string kind | sourceModel(_, _, _, _, _, _, _, kind, _) | - not kind = ["remote", "contentprovider", "android-widget", "android-external-storage-dir"] and - not kind.matches("qltest%") and - result = "Invalid kind \"" + kind + "\" in source model." - ) + // All source kinds are supported, but some may not be used in this query. + // or + // exists(string kind | sourceModel(_, _, _, _, _, _, _, kind, _) | + // not ConfiguredExtensions::supportedSourceKind(kind) and + // not kind.matches("qltest%") and + // result = "Invalid kind \"" + kind + "\" in source model." + // ) } private string getInvalidModelSignature() { diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll new file mode 100644 index 00000000000..cca6e2f97ad --- /dev/null +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlowConfiguration.qll @@ -0,0 +1,59 @@ +/** + * This module provides extensible predicates for configuring which kinds of MaD models + * are applicable to a given query. + */ + +import ExternalFlowExtensions + +/** + * Holds if the specified kind of source model is supported for the current query. + */ +extensible predicate supportedThreatModel(string kind); + +/** + * Holds if the specified kind of source model is containted within the specified group. + */ +extensible predicate threatModelGrouping(string kind, string group); + + +/** + * Finds all of the threat models that are ancestors of the specified kind. + */ +private string parentThreatModel(string kind) { + threatModelGrouping(kind, result) + or exists(string parent | parentThreatModel(kind) = parent | + threatModelGrouping(parent, result) + ) +} + +/** + * Finds all of the threat models that are children of the specified group. + */ +private string childThreatModel(string group) { + threatModelGrouping(result, group) + or exists(string child | childThreatModel(group) = child | + threatModelGrouping(result, child) + ) +} + + /** + * Holds if source models of the specified kind are + * supported for the current query. + */ +bindingset[kind] +predicate supportedSourceModel(string kind) { + // expansive threat model includes all kinds + supportedThreatModel("expansive") + or + // check if this kind is supported directly + supportedThreatModel(kind) + or + // check if one of this kind's ancestors are supported + exists(string group | + group = parentThreatModel(kind) | + supportedThreatModel(group) + ) + or + // if supportedThreatModel is empty, check if kind is a subtype of "standard" + count(any(string s | supportedThreatModel(s))) = 0 and ("standard" = parentThreatModel(kind) or "standard" = kind) +} diff --git a/java/ql/src/t.ql b/java/ql/src/t.ql new file mode 100644 index 00000000000..3dfab05f39b --- /dev/null +++ b/java/ql/src/t.ql @@ -0,0 +1,23 @@ +import java +import semmle.code.java.dataflow.ExternalFlowConfiguration + + +string kind() { + result = [ + "remote", + "local", + "android", + "sql", + "android-external-storage-dir", + "standard", + "expansive", + "hucairz", + "request", + "response", + ] +} + +from string a, string b +where (a = kind()) + and ((supportedSourceModel(a) and b = "YES") or (not supportedSourceModel(a) and b = "NO")) +select a, b diff --git a/java/ql/test/experimental/configured-flow/Test.java b/java/ql/test/experimental/configured-flow/Test.java new file mode 100644 index 00000000000..391b657956e --- /dev/null +++ b/java/ql/test/experimental/configured-flow/Test.java @@ -0,0 +1,28 @@ +import java.sql.*; +import java.net.*; + +class Test { + public static void main(String[] args) throws Exception { + Connection conn = DriverManager.getConnection(""); + Statement readStmt = conn.createStatement(); + Statement writeStmt = conn.createStatement(); + Socket sock = new Socket("localhost", 1234); + + // Source only if "sql" is a selected threat model + ResultSet rs = readStmt.executeQuery("SELECT * FROM foo"); + + // Only a source if "remote" is a selected threat model + int val = sock.getInputStream().read(); + + // Sink + sock.getOutputStream().write(val); + + // Sink + writeStmt.executeUpdate("INSERT INTO foo VALUES ('" + val + "')"); + + // Sink + writeStmt.executeUpdate("INSERT INTO foo VALUES ('" + rs.getString("name") + "')"); + // Sink + sock.getOutputStream().write(rs.getString("name").getBytes()); + } +} diff --git a/java/ql/test/experimental/configured-flow/codeql-pack.yml b/java/ql/test/experimental/configured-flow/codeql-pack.yml new file mode 100644 index 00000000000..516153eceab --- /dev/null +++ b/java/ql/test/experimental/configured-flow/codeql-pack.yml @@ -0,0 +1,9 @@ +name: test/configured-threat-models +version: 0.0.0 +dependencies: + codeql/java-all: "*" + +dataExtensions: + - ext/*.yml + +extractor: java diff --git a/java/ql/test/experimental/configured-flow/ext/threat-models.yml b/java/ql/test/experimental/configured-flow/ext/threat-models.yml new file mode 100644 index 00000000000..8e081c3ad1a --- /dev/null +++ b/java/ql/test/experimental/configured-flow/ext/threat-models.yml @@ -0,0 +1,8 @@ +extensions: + + - addsTo: + pack: codeql/java-all + extensible: supportedThreatModel + data: + - ["remote"] + - ["sql"] diff --git a/java/ql/test/experimental/configured-flow/test.expected b/java/ql/test/experimental/configured-flow/test.expected new file mode 100644 index 00000000000..89d94607e73 --- /dev/null +++ b/java/ql/test/experimental/configured-flow/test.expected @@ -0,0 +1,26 @@ +edges +| Test.java:12:20:12:61 | executeQuery(...) : ResultSet | Test.java:24:59:24:60 | rs : ResultSet | +| Test.java:12:20:12:61 | executeQuery(...) : ResultSet | Test.java:26:34:26:35 | rs : ResultSet | +| Test.java:15:15:15:42 | read(...) : Number | Test.java:18:34:18:36 | val | +| Test.java:15:15:15:42 | read(...) : Number | Test.java:21:29:21:68 | ... + ... | +| Test.java:24:59:24:60 | rs : ResultSet | Test.java:24:59:24:78 | getString(...) : String | +| Test.java:24:59:24:78 | getString(...) : String | Test.java:24:29:24:85 | ... + ... | +| Test.java:26:34:26:35 | rs : ResultSet | Test.java:26:34:26:53 | getString(...) : String | +| Test.java:26:34:26:53 | getString(...) : String | Test.java:26:34:26:64 | getBytes(...) | +nodes +| Test.java:12:20:12:61 | executeQuery(...) : ResultSet | semmle.label | executeQuery(...) : ResultSet | +| Test.java:15:15:15:42 | read(...) : Number | semmle.label | read(...) : Number | +| Test.java:18:34:18:36 | val | semmle.label | val | +| Test.java:21:29:21:68 | ... + ... | semmle.label | ... + ... | +| Test.java:24:29:24:85 | ... + ... | semmle.label | ... + ... | +| Test.java:24:59:24:60 | rs : ResultSet | semmle.label | rs : ResultSet | +| Test.java:24:59:24:78 | getString(...) : String | semmle.label | getString(...) : String | +| Test.java:26:34:26:35 | rs : ResultSet | semmle.label | rs : ResultSet | +| Test.java:26:34:26:53 | getString(...) : String | semmle.label | getString(...) : String | +| Test.java:26:34:26:64 | getBytes(...) | semmle.label | getBytes(...) | +subpaths +#select +| Test.java:18:34:18:36 | val | Test.java:15:15:15:42 | read(...) : Number | Test.java:18:34:18:36 | val | This is some kind of threat model thingy $@. | Test.java:15:15:15:42 | read(...) | Source of that thingy | +| Test.java:21:29:21:68 | ... + ... | Test.java:15:15:15:42 | read(...) : Number | Test.java:21:29:21:68 | ... + ... | This is some kind of threat model thingy $@. | Test.java:15:15:15:42 | read(...) | Source of that thingy | +| Test.java:24:29:24:85 | ... + ... | Test.java:12:20:12:61 | executeQuery(...) : ResultSet | Test.java:24:29:24:85 | ... + ... | This is some kind of threat model thingy $@. | Test.java:12:20:12:61 | executeQuery(...) | Source of that thingy | +| Test.java:26:34:26:64 | getBytes(...) | Test.java:12:20:12:61 | executeQuery(...) : ResultSet | Test.java:26:34:26:64 | getBytes(...) | This is some kind of threat model thingy $@. | Test.java:12:20:12:61 | executeQuery(...) | Source of that thingy | diff --git a/java/ql/test/experimental/configured-flow/test.ql b/java/ql/test/experimental/configured-flow/test.ql new file mode 100644 index 00000000000..cfdb114d8f5 --- /dev/null +++ b/java/ql/test/experimental/configured-flow/test.ql @@ -0,0 +1,40 @@ +/** + * @name Testing the threat model + * @kind path-problem + * @problem.severity warning + * @precision low + * @id java/threat-model + * @tags security + */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.ExternalFlow +import ThreatModel::PathGraph +import semmle.code.java.dataflow.TaintTracking + +private module ThreatModelConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + sourceNode(source, _) + } + + predicate isSink(DataFlow::Node sink) { + sinkNode(sink, _) + } +} + +module ThreatModel = TaintTracking::Global; + +from ThreatModel::PathNode source, ThreatModel::PathNode sink +where ThreatModel::flowPath(source, sink) +select sink.getNode(), source, sink, "This is some kind of threat model thingy $@.", source.getNode(), + "Source of that thingy" + +// from DataFlow::Node source, DataFlow::Node sink +// where ThreatModel::flow(source, sink) +// select source, sink + + +// from DataFlow::Node node, string kind +// where sourceNode(node, kind) +// select node, kind diff --git a/java/ql/test/library-tests/dataflow/external-models/srcs.ext.yml b/java/ql/test/library-tests/dataflow/external-models/srcs.ext.yml index 7730d41e549..c9ff7ec31f9 100644 --- a/java/ql/test/library-tests/dataflow/external-models/srcs.ext.yml +++ b/java/ql/test/library-tests/dataflow/external-models/srcs.ext.yml @@ -19,3 +19,21 @@ extensions: - ["my.qltest", "A$Tag", False, "", "", "Annotated", "", "qltest-nospec", "manual"] - ["my.qltest", "A", False, "srcTwoArg", "(String,String)", "", "ReturnValue", "qltest-shortsig", "manual"] - ["my.qltest", "A", False, "srcTwoArg", "(java.lang.String,java.lang.String)", "", "ReturnValue", "qltest-longsig", "manual"] + + - addsTo: + pack: codeql/java-all + extensible: supportedThreatModel + data: + - ["standard"] + - ["qltest"] + - ["qltest-alt"] + - ["qltest-w-subtypes"] + - ["qltest-argany"] + - ["qltest-all-overloads"] + - ["qltest-argnum"] + - ["qltest-retval"] + - ["qltest-param"] + - ["qltest-nospec"] + - ["qltest-shortsig"] + - ["qltest-longsig"] + - ["qltest-param-override"]