Skip to content

Commit 4ec4da4

Browse files
aschackmullaschackmull
committed
Dataflow/Java: Add support for pretty-printed provenace in tests. Convert one test.
1 parent 68a78fa commit 4ec4da4

File tree

6 files changed

+147
-8
lines changed

6 files changed

+147
-8
lines changed

‎java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,44 @@ predicate summaryModel(
185185
)
186186
}
187187

188+
/**
189+
* Holds if the given extension tuple `madId` should pretty-print as `model`.
190+
*
191+
* This predicate should only be used in tests.
192+
*/
193+
predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
194+
exists(
195+
string package, string type, boolean subtypes, string name, string signature, string ext,
196+
string output, string kind, string provenance
197+
|
198+
sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance, madId) and
199+
model =
200+
"Source: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
201+
ext + "; " + output + "; " + kind + "; " + provenance
202+
)
203+
or
204+
exists(
205+
string package, string type, boolean subtypes, string name, string signature, string ext,
206+
string input, string kind, string provenance
207+
|
208+
sinkModel(package, type, subtypes, name, signature, ext, input, kind, provenance, madId) and
209+
model =
210+
"Sink: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
211+
ext + "; " + input + "; " + kind + "; " + provenance
212+
)
213+
or
214+
exists(
215+
string package, string type, boolean subtypes, string name, string signature, string ext,
216+
string input, string output, string kind, string provenance
217+
|
218+
summaryModel(package, type, subtypes, name, signature, ext, input, output, kind, provenance,
219+
madId) and
220+
model =
221+
"Summary: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
222+
ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
223+
)
224+
}
225+
188226
/** Holds if a neutral model exists for the given parameters. */
189227
predicate neutralModel = Extensions::neutralModel/6;
190228

‎java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.expected

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
1+
models
2+
| 1 | Sink: java.net; URL; false; openConnection; ; ; Argument[this]; request-forgery; manual |
3+
| 2 | Summary: java.net; URL; false; URL; (String); ; Argument[0]; Argument[this]; taint; manual |
4+
| 3 | Summary: java.net; URL; false; URL; (URL,String); ; Argument[1]; Argument[this]; taint; ai-manual |
15
edges
26
| HttpsUrlsTest.java:23:23:23:31 | "http://" : String | HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | provenance | |
3-
| HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | HttpsUrlsTest.java:28:50:28:50 | u | provenance | Sink:MaD:42944 |
7+
| HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | HttpsUrlsTest.java:28:50:28:50 | u | provenance | Sink:MaD:1 |
48
| HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | Config |
5-
| HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | MaD:42977 |
9+
| HttpsUrlsTest.java:24:21:24:56 | ... + ... : String | HttpsUrlsTest.java:24:13:24:57 | new URL(...) : URL | provenance | MaD:2 |
610
| HttpsUrlsTest.java:36:23:36:28 | "http" : String | HttpsUrlsTest.java:37:21:37:28 | protocol : String | provenance | |
7-
| HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | HttpsUrlsTest.java:41:50:41:50 | u | provenance | Sink:MaD:42944 |
11+
| HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | HttpsUrlsTest.java:41:50:41:50 | u | provenance | Sink:MaD:1 |
812
| HttpsUrlsTest.java:37:21:37:28 | protocol : String | HttpsUrlsTest.java:37:13:37:62 | new URL(...) : URL | provenance | Config |
913
| HttpsUrlsTest.java:49:23:49:31 | "http://" : String | HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | provenance | |
10-
| HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | HttpsUrlsTest.java:55:50:55:50 | u | provenance | Sink:MaD:42944 |
14+
| HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | HttpsUrlsTest.java:55:50:55:50 | u | provenance | Sink:MaD:1 |
1115
| HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | Config |
12-
| HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | MaD:42985 |
16+
| HttpsUrlsTest.java:51:64:51:98 | ... + ... : String | HttpsUrlsTest.java:51:13:51:99 | new URL(...) : URL | provenance | MaD:3 |
1317
| HttpsUrlsTest.java:87:23:87:28 | "http" : String | HttpsUrlsTest.java:88:21:88:28 | protocol : String | provenance | |
14-
| HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | HttpsUrlsTest.java:92:50:92:50 | u | provenance | Sink:MaD:42944 |
18+
| HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | HttpsUrlsTest.java:92:50:92:50 | u | provenance | Sink:MaD:1 |
1519
| HttpsUrlsTest.java:88:21:88:28 | protocol : String | HttpsUrlsTest.java:88:13:88:52 | new URL(...) : URL | provenance | Config |
1620
nodes
1721
| HttpsUrlsTest.java:23:23:23:31 | "http://" : String | semmle.label | "http://" : String |

‎java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import java
6+
import semmle.code.java.security.HttpsUrlsQuery
7+
import codeql.dataflow.test.ProvenancePathGraph
8+
import semmle.code.java.dataflow.ExternalFlow
9+
import ShowProvenance<interpretModelForTest/2, HttpStringToUrlOpenMethodFlow::PathNode, HttpStringToUrlOpenMethodFlow::PathGraph>
10+
11+
from HttpStringToUrlOpenMethodFlow::PathNode source, HttpStringToUrlOpenMethodFlow::PathNode sink
12+
where HttpStringToUrlOpenMethodFlow::flowPath(source, sink)
13+
select sink.getNode(), source, sink, "URL may have been constructed with HTTP protocol, using $@.",
14+
source.getNode(), "this HTTP URL"

‎java/ql/test/query-tests/security/CWE-311/CWE-319/HttpsUrls.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3905,10 +3905,12 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
39053905
final predicate isSinkGroup(string group) { this = TPathNodeSinkGroup(group) }
39063906
}
39073907

3908+
private import codeql.dataflow.test.ProvenancePathGraph as ProvenancePathGraph
3909+
39083910
/**
39093911
* Provides the query predicates needed to include a graph in a path-problem query.
39103912
*/
3911-
module PathGraph implements PathGraphSig<PathNode> {
3913+
module PathGraph implements PathGraphSig<PathNode>, ProvenancePathGraph::PathGraphSig<PathNode> {
39123914
/** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
39133915
query predicate edges(PathNode a, PathNode b, string key, string val) {
39143916
a.(PathNodeImpl).getANonHiddenSuccessor(val) = b and

‎shared/dataflow/codeql/dataflow/test/ProvenancePathGraph.qll

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Provides a module for renumbering MaD IDs in data flow path explanations in
3+
* order to produce more stable test output.
4+
*
5+
* In addition to the `PathGraph`, a `query predicate models` is provided to
6+
* list the contents of the referenced MaD rows.
7+
*/
8+
signature predicate interpretModelForTestSig(QlBuiltins::ExtensionId madId, string model);
9+
10+
signature class PathNodeSig {
11+
string toString();
12+
}
13+
14+
signature module PathGraphSig<PathNodeSig PathNode> {
15+
/** Holds if `(a,b)` is an edge in the graph of data flow path explanations. */
16+
predicate edges(PathNode a, PathNode b, string key, string val);
17+
18+
/** Holds if `n` is a node in the graph of data flow path explanations. */
19+
predicate nodes(PathNode n, string key, string val);
20+
21+
/**
22+
* Holds if `(arg, par, ret, out)` forms a subpath-tuple, that is, flow through
23+
* a subpath between `par` and `ret` with the connecting edges `arg -> par` and
24+
* `ret -> out` is summarized as the edge `arg -> out`.
25+
*/
26+
predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out);
27+
}
28+
29+
module ShowProvenance<
30+
interpretModelForTestSig/2 interpretModelForTest, PathNodeSig PathNode,
31+
PathGraphSig<PathNode> PathGraph>
32+
{
33+
private predicate madIds(string madId) {
34+
exists(string model |
35+
PathGraph::edges(_, _, _, model) and
36+
model.regexpFind("(?<=MaD:)[0-9]*", _, _) = madId
37+
)
38+
}
39+
40+
private predicate rankedMadIds(string madId, int r) {
41+
madId = rank[r](string madId0 | madIds(madId0) | madId0 order by madId0.toInt())
42+
}
43+
44+
query predicate models(int r, string model) {
45+
exists(QlBuiltins::ExtensionId madId |
46+
rankedMadIds(madId.toString(), r) and interpretModelForTest(madId, model)
47+
)
48+
}
49+
50+
private predicate translateModelsPart(string model1, string model2, int i) {
51+
PathGraph::edges(_, _, _, model1) and
52+
exists(string s | model1.splitAt("MaD:", i) = s |
53+
model2 = s and i = 0
54+
or
55+
exists(string part, string madId, string rest, int r |
56+
translateModelsPart(model1, part, i - 1) and
57+
madId = s.regexpCapture("([0-9]*)(.*)", 1) and
58+
rest = s.regexpCapture("([0-9]*)(.*)", 2) and
59+
rankedMadIds(madId, r) and
60+
model2 = part + "MaD:" + r + rest
61+
)
62+
)
63+
}
64+
65+
private predicate translateModels(string model1, string model2) {
66+
exists(int i |
67+
translateModelsPart(model1, model2, i) and
68+
not translateModelsPart(model1, _, i + 1)
69+
)
70+
}
71+
72+
query predicate edges(PathNode a, PathNode b, string key, string val) {
73+
exists(string model |
74+
PathGraph::edges(a, b, key, model) and
75+
translateModels(model, val)
76+
)
77+
}
78+
79+
query predicate nodes = PathGraph::nodes/3;
80+
81+
query predicate subpaths = PathGraph::subpaths/4;
82+
}

0 commit comments

Comments
 (0)