Dataflow/Java: Add support for pretty-printed provenace in tests. Convert one test.

2024-04-15 10:48:26 +02:00 · 2024-04-15 10:48:26 +02:00 · 4ec4da4c8c
--- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
+++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
@ -185,6 +185,44 @@ predicate summaryModel(
  )
 }

+/**
+ * Holds if the given extension tuple `madId` should pretty-print as `model`.
+ *
+ * This predicate should only be used in tests.
+ */
+predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
+  exists(
+    string package, string type, boolean subtypes, string name, string signature, string ext,
+    string output, string kind, string provenance
+  |
+    sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance, madId) and
+    model =
+      "Source: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
+        ext + "; " + output + "; " + kind + "; " + provenance
+  )
+  or
+  exists(
+    string package, string type, boolean subtypes, string name, string signature, string ext,
+    string input, string kind, string provenance
+  |
+    sinkModel(package, type, subtypes, name, signature, ext, input, kind, provenance, madId) and
+    model =
+      "Sink: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
+        ext + "; " + input + "; " + kind + "; " + provenance
+  )
+  or
+  exists(
+    string package, string type, boolean subtypes, string name, string signature, string ext,
+    string input, string output, string kind, string provenance
+  |
+    summaryModel(package, type, subtypes, name, signature, ext, input, output, kind, provenance,
+      madId) and
+    model =
+      "Summary: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
+        ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
+  )
+}
+
 /** Holds if a neutral model exists for the given parameters. */
 predicate neutralModel = Extensions::neutralModel/6;

--- a/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.expected
+++ b/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.expected
@ -1,17 +1,21 @@
+models
+| 1 | Sink: java.net; URL; false; openConnection; ; ; Argument[this]; request-forgery; manual |
+| 2 | Summary: java.net; URL; false; URL; (String); ; Argument[0]; Argument[this]; taint; manual |
+| 3 | Summary: java.net; URL; false; URL; (URL,String); ; Argument[1]; Argument[this]; taint; ai-manual |
 edges
 | HttpsUrlsTest.java:23:23:23:31 | "http://" : String | HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | provenance |  |
-| HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | HttpsUrlsTest.java:28:50:28:50 | u | provenance |  Sink:MaD:42944 |
+| HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | HttpsUrlsTest.java:28:50:28:50 | u | provenance |  Sink:MaD:1 |
 | HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | Config |
-| HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | MaD:42977 |
+| HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | MaD:2 |
 | HttpsUrlsTest.java:36:23:36:28 | "http" : String | HttpsUrlsTest.java:37:21:37:28 | protocol : String | provenance |  |
-| HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | HttpsUrlsTest.java:41:50:41:50 | u | provenance |  Sink:MaD:42944 |
+| HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | HttpsUrlsTest.java:41:50:41:50 | u | provenance |  Sink:MaD:1 |
 | HttpsUrlsTest.java:37:21:37:28 | protocol : String | HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | provenance | Config |
 | HttpsUrlsTest.java:49:23:49:31 | "http://" : String | HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | provenance |  |
-| HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | HttpsUrlsTest.java:55:50:55:50 | u | provenance |  Sink:MaD:42944 |
+| HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | HttpsUrlsTest.java:55:50:55:50 | u | provenance |  Sink:MaD:1 |
 | HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | Config |
-| HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | MaD:42985 |
+| HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | MaD:3 |
 | HttpsUrlsTest.java:87:23:87:28 | "http" : String | HttpsUrlsTest.java:88:21:88:28 | protocol : String | provenance |  |
-| HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | HttpsUrlsTest.java:92:50:92:50 | u | provenance |  Sink:MaD:42944 |
+| HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | HttpsUrlsTest.java:92:50:92:50 | u | provenance |  Sink:MaD:1 |
 | HttpsUrlsTest.java:88:21:88:28 | protocol : String | HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | provenance | Config |
 nodes
 | HttpsUrlsTest.java:23:23:23:31 | "http://" : String | semmle.label | "http://" : String |
--- a/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.ql
+++ b/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.ql
@ -0,0 +1,14 @@
+/**
+ * @kind path-problem
+ */
+
+import java
+import semmle.code.java.security.HttpsUrlsQuery
+import codeql.dataflow.test.ProvenancePathGraph
+import semmle.code.java.dataflow.ExternalFlow
+import ShowProvenance<interpretModelForTest/2, HttpStringToUrlOpenMethodFlow::PathNode, HttpStringToUrlOpenMethodFlow::PathGraph>
+
+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"
--- a/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.qlref
+++ b/java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.qlref
@ -1 +0,0 @@
-Security/CWE/CWE-319/HttpsUrls.ql
--- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
+++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@ -3905,10 +3905,12 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
      final predicate isSinkGroup(string group) { this = TPathNodeSinkGroup(group) }
    }

+    private import codeql.dataflow.test.ProvenancePathGraph as ProvenancePathGraph
+
    /**
     * Provides the query predicates needed to include a graph in a path-problem query.
     */
-    module PathGraph implements PathGraphSig<PathNode> {
+    module PathGraph implements PathGraphSig<PathNode>, ProvenancePathGraph::PathGraphSig<PathNode> {
      /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
      query predicate edges(PathNode a, PathNode b, string key, string val) {
        a.(PathNodeImpl).getANonHiddenSuccessor(val) = b and
--- a/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll
+++ b/shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll
@ -0,0 +1,82 @@
+/**
+ * Provides a module for renumbering MaD IDs in data flow path explanations in
+ * order to produce more stable test output.
+ *
+ * In addition to the `PathGraph`, a `query predicate models` is provided to
+ * list the contents of the referenced MaD rows.
+ */
+signature predicate interpretModelForTestSig(QlBuiltins::ExtensionId madId, string model);
+
+signature class PathNodeSig {
+  string toString();
+}
+
+signature module PathGraphSig<PathNodeSig PathNode> {
+  /** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
+  predicate edges(PathNode a, PathNode b, string key, string val);
+
+  /** Holds if `n` is a node in the graph of data flow path explanations. */
+  predicate nodes(PathNode n, string key, string val);
+
+  /**
+   * Holds if `(arg, par, ret, out)` forms a subpath-tuple, that is, flow through
+   * a subpath between `par` and `ret` with the connecting edges `arg -> par` and
+   * `ret -> out` is summarized as the edge `arg -> out`.
+   */
+  predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out);
+}
+
+module ShowProvenance<
+  interpretModelForTestSig/2 interpretModelForTest, PathNodeSig PathNode,
+  PathGraphSig<PathNode> PathGraph>
+{
+  private predicate madIds(string madId) {
+    exists(string model |
+      PathGraph::edges(_, _, _, model) and
+      model.regexpFind("(?<=MaD:)[0-9]*", _, _) = madId
+    )
+  }
+
+  private predicate rankedMadIds(string madId, int r) {
+    madId = rank[r](string madId0 | madIds(madId0) | madId0 order by madId0.toInt())
+  }
+
+  query predicate models(int r, string model) {
+    exists(QlBuiltins::ExtensionId madId |
+      rankedMadIds(madId.toString(), r) and interpretModelForTest(madId, model)
+    )
+  }
+
+  private predicate translateModelsPart(string model1, string model2, int i) {
+    PathGraph::edges(_, _, _, model1) and
+    exists(string s | model1.splitAt("MaD:", i) = s |
+      model2 = s and i = 0
+      or
+      exists(string part, string madId, string rest, int r |
+        translateModelsPart(model1, part, i - 1) and
+        madId = s.regexpCapture("([0-9]*)(.*)", 1) and
+        rest = s.regexpCapture("([0-9]*)(.*)", 2) and
+        rankedMadIds(madId, r) and
+        model2 = part + "MaD:" + r + rest
+      )
+    )
+  }
+
+  private predicate translateModels(string model1, string model2) {
+    exists(int i |
+      translateModelsPart(model1, model2, i) and
+      not translateModelsPart(model1, _, i + 1)
+    )
+  }
+
+  query predicate edges(PathNode a, PathNode b, string key, string val) {
+    exists(string model |
+      PathGraph::edges(a, b, key, model) and
+      translateModels(model, val)
+    )
+  }
+
+  query predicate nodes = PathGraph::nodes/3;
+
+  query predicate subpaths = PathGraph::subpaths/4;
+}