Skip to content

Commit 32260e2

Browse files
aschackmullaschackmull
authored
Merge pull request #16210 from aschackmull/dataflow/provenance-for-tests
Dataflow: Add support for pretty-printed alert provenance in tests
2 parents ea3a3db + 9b1e4d7 commit 32260e2

File tree

20 files changed

+355
-22
lines changed

20 files changed

+355
-22
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,44 @@ private import semmle.code.csharp.dispatch.OverridableCallable
9898
private import semmle.code.csharp.frameworks.System
9999
private import codeql.mad.ModelValidation as SharedModelVal
100100

101+
/**
102+
* Holds if the given extension tuple `madId` should pretty-print as `model`.
103+
*
104+
* This predicate should only be used in tests.
105+
*/
106+
predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
107+
exists(
108+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
109+
string output, string kind, string provenance
110+
|
111+
sourceModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, madId) and
112+
model =
113+
"Source: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; "
114+
+ ext + "; " + output + "; " + kind + "; " + provenance
115+
)
116+
or
117+
exists(
118+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
119+
string input, string kind, string provenance
120+
|
121+
sinkModel(namespace, type, subtypes, name, signature, ext, input, kind, provenance, madId) and
122+
model =
123+
"Sink: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
124+
ext + "; " + input + "; " + kind + "; " + provenance
125+
)
126+
or
127+
exists(
128+
string namespace, string type, boolean subtypes, string name, string signature, string ext,
129+
string input, string output, string kind, string provenance
130+
|
131+
summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance,
132+
madId) and
133+
model =
134+
"Summary: " + namespace + "; " + type + "; " + subtypes + "; " + name + "; " + signature +
135+
"; " + ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
136+
)
137+
}
138+
101139
private predicate relevantNamespace(string namespace) {
102140
sourceModel(namespace, _, _, _, _, _, _, _, _, _) or
103141
sinkModel(namespace, _, _, _, _, _, _, _, _, _) or

‎csharp/ql/test/query-tests/Security Features/CWE-807/ConditionalBypass.expected‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,32 @@
1+
models
2+
| 1 | Summary: System.Net; IPHostEntry; false; get_HostName; (); ; Argument[this]; ReturnValue; taint; manual |
3+
| 2 | Summary: System.Web; HttpCookie; false; get_Value; (); ; Argument[this]; ReturnValue; taint; manual |
4+
| 3 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
15
edges
26
| ConditionalBypass.cs:12:16:12:22 | access to local variable isAdmin : String | ConditionalBypass.cs:16:13:16:30 | ... == ... | provenance | |
37
| ConditionalBypass.cs:12:26:12:48 | access to property QueryString : NameValueCollection | ConditionalBypass.cs:12:16:12:22 | access to local variable isAdmin : String | provenance | |
4-
| ConditionalBypass.cs:12:26:12:48 | access to property QueryString : NameValueCollection | ConditionalBypass.cs:12:26:12:59 | access to indexer : String | provenance | MaD:11390 |
8+
| ConditionalBypass.cs:12:26:12:48 | access to property QueryString : NameValueCollection | ConditionalBypass.cs:12:26:12:59 | access to indexer : String | provenance | MaD:3 |
59
| ConditionalBypass.cs:12:26:12:59 | access to indexer : String | ConditionalBypass.cs:12:16:12:22 | access to local variable isAdmin : String | provenance | |
610
| ConditionalBypass.cs:19:20:19:30 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:22:13:22:23 | access to local variable adminCookie : HttpCookie | provenance | |
711
| ConditionalBypass.cs:19:20:19:30 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:27:13:27:23 | access to local variable adminCookie : HttpCookie | provenance | |
812
| ConditionalBypass.cs:19:34:19:52 | access to property Cookies : HttpCookieCollection | ConditionalBypass.cs:19:20:19:30 | access to local variable adminCookie : HttpCookie | provenance | |
9-
| ConditionalBypass.cs:22:13:22:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:22:13:22:29 | access to property Value : String | provenance | MaD:2161 |
13+
| ConditionalBypass.cs:22:13:22:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:22:13:22:29 | access to property Value : String | provenance | MaD:2 |
1014
| ConditionalBypass.cs:22:13:22:29 | access to property Value : String | ConditionalBypass.cs:22:13:22:45 | call to method Equals | provenance | |
11-
| ConditionalBypass.cs:27:13:27:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:27:13:27:29 | access to property Value : String | provenance | MaD:2161 |
15+
| ConditionalBypass.cs:27:13:27:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:27:13:27:29 | access to property Value : String | provenance | MaD:2 |
1216
| ConditionalBypass.cs:27:13:27:29 | access to property Value : String | ConditionalBypass.cs:27:13:27:40 | ... == ... | provenance | |
1317
| ConditionalBypass.cs:42:21:42:28 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:44:13:44:20 | access to local variable hostInfo : IPHostEntry | provenance | |
1418
| ConditionalBypass.cs:42:21:42:28 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:49:13:49:20 | access to local variable hostInfo : IPHostEntry | provenance | |
1519
| ConditionalBypass.cs:42:32:42:66 | call to method GetHostByAddress : IPHostEntry | ConditionalBypass.cs:42:21:42:28 | access to local variable hostInfo : IPHostEntry | provenance | |
16-
| ConditionalBypass.cs:44:13:44:20 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:44:13:44:29 | access to property HostName : String | provenance | MaD:1827 |
20+
| ConditionalBypass.cs:44:13:44:20 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:44:13:44:29 | access to property HostName : String | provenance | MaD:1 |
1721
| ConditionalBypass.cs:44:13:44:29 | access to property HostName : String | ConditionalBypass.cs:44:13:44:46 | ... == ... | provenance | |
18-
| ConditionalBypass.cs:49:13:49:20 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:49:13:49:29 | access to property HostName | provenance | MaD:1827 |
22+
| ConditionalBypass.cs:49:13:49:20 | access to local variable hostInfo : IPHostEntry | ConditionalBypass.cs:49:13:49:29 | access to property HostName | provenance | MaD:1 |
1923
| ConditionalBypass.cs:70:20:70:30 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:72:13:72:23 | access to local variable adminCookie : HttpCookie | provenance | |
2024
| ConditionalBypass.cs:70:34:70:52 | access to property Cookies : HttpCookieCollection | ConditionalBypass.cs:70:20:70:30 | access to local variable adminCookie : HttpCookie | provenance | |
21-
| ConditionalBypass.cs:72:13:72:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:72:13:72:29 | access to property Value : String | provenance | MaD:2161 |
25+
| ConditionalBypass.cs:72:13:72:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:72:13:72:29 | access to property Value : String | provenance | MaD:2 |
2226
| ConditionalBypass.cs:72:13:72:29 | access to property Value : String | ConditionalBypass.cs:72:13:72:40 | ... == ... | provenance | |
2327
| ConditionalBypass.cs:83:20:83:30 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:84:13:84:23 | access to local variable adminCookie : HttpCookie | provenance | |
2428
| ConditionalBypass.cs:83:34:83:52 | access to property Cookies : HttpCookieCollection | ConditionalBypass.cs:83:20:83:30 | access to local variable adminCookie : HttpCookie | provenance | |
25-
| ConditionalBypass.cs:84:13:84:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:84:13:84:29 | access to property Value : String | provenance | MaD:2161 |
29+
| ConditionalBypass.cs:84:13:84:23 | access to local variable adminCookie : HttpCookie | ConditionalBypass.cs:84:13:84:29 | access to property Value : String | provenance | MaD:2 |
2630
| ConditionalBypass.cs:84:13:84:29 | access to property Value : String | ConditionalBypass.cs:84:13:84:40 | ... == ... | provenance | |
2731
nodes
2832
| ConditionalBypass.cs:12:16:12:22 | access to local variable isAdmin : String | semmle.label | access to local variable isAdmin : String |

‎csharp/ql/test/query-tests/Security Features/CWE-807/ConditionalBypass.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 csharp
6+
import semmle.code.csharp.security.dataflow.ConditionalBypassQuery
7+
import codeql.dataflow.test.ProvenancePathGraph
8+
import semmle.code.csharp.dataflow.internal.ExternalFlow
9+
import ShowProvenance<interpretModelForTest/2, ConditionalBypass::PathNode, ConditionalBypass::PathGraph>
10+
11+
from ConditionalBypass::PathNode source, ConditionalBypass::PathNode sink
12+
where ConditionalBypass::flowPath(source, sink)
13+
select sink.getNode(), source, sink, "This condition guards a sensitive $@, but a $@ controls it.",
14+
sink.getNode().(Sink).getSensitiveMethodCall(), "action", source.getNode(), "user-provided value"

‎csharp/ql/test/query-tests/Security Features/CWE-807/ConditionalBypass.qlref‎

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

‎go/ql/lib/semmle/go/dataflow/ExternalFlow.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,44 @@ private import internal.FlowSummaryImpl::Private
8484
private import internal.FlowSummaryImpl::Private::External
8585
private import codeql.mad.ModelValidation as SharedModelVal
8686

87+
/**
88+
* Holds if the given extension tuple `madId` should pretty-print as `model`.
89+
*
90+
* This predicate should only be used in tests.
91+
*/
92+
predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
93+
exists(
94+
string package, string type, boolean subtypes, string name, string signature, string ext,
95+
string output, string kind, string provenance
96+
|
97+
sourceModel(package, type, subtypes, name, signature, ext, output, kind, provenance, madId) and
98+
model =
99+
"Source: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
100+
ext + "; " + output + "; " + kind + "; " + provenance
101+
)
102+
or
103+
exists(
104+
string package, string type, boolean subtypes, string name, string signature, string ext,
105+
string input, string kind, string provenance
106+
|
107+
sinkModel(package, type, subtypes, name, signature, ext, input, kind, provenance, madId) and
108+
model =
109+
"Sink: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
110+
ext + "; " + input + "; " + kind + "; " + provenance
111+
)
112+
or
113+
exists(
114+
string package, string type, boolean subtypes, string name, string signature, string ext,
115+
string input, string output, string kind, string provenance
116+
|
117+
summaryModel(package, type, subtypes, name, signature, ext, input, output, kind, provenance,
118+
madId) and
119+
model =
120+
"Summary: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
121+
ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
122+
)
123+
}
124+
87125
private predicate relevantPackage(string package) {
88126
sourceModel(package, _, _, _, _, _, _, _, _, _) or
89127
sinkModel(package, _, _, _, _, _, _, _, _, _) or

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.expected‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
models
2+
| 1 | Summary: net/url; URL; true; Query; ; ; Argument[receiver]; ReturnValue; taint; manual |
3+
| 2 | Summary: path; ; false; Clean; ; ; Argument[0]; ReturnValue; taint; manual |
14
edges
2-
| TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:14:18:14:30 | call to Query | provenance | MaD:735 |
5+
| TaintedPath.go:14:18:14:22 | selection of URL | TaintedPath.go:14:18:14:30 | call to Query | provenance | MaD:1 |
36
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:17:29:17:40 | tainted_path | provenance | |
47
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:21:57:21:68 | tainted_path | provenance | |
58
| TaintedPath.go:14:18:14:30 | call to Query | TaintedPath.go:68:39:68:56 | ...+... | provenance | |
69
| TaintedPath.go:21:57:21:68 | tainted_path | TaintedPath.go:21:28:21:69 | call to Join | provenance | FunctionModel |
7-
| TaintedPath.go:68:39:68:56 | ...+... | TaintedPath.go:68:28:68:57 | call to Clean | provenance | MaD:761 |
10+
| TaintedPath.go:68:39:68:56 | ...+... | TaintedPath.go:68:28:68:57 | call to Clean | provenance | MaD:2 |
811
nodes
912
| TaintedPath.go:14:18:14:22 | selection of URL | semmle.label | selection of URL |
1013
| TaintedPath.go:14:18:14:30 | call to Query | semmle.label | call to Query |

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.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 go
6+
import semmle.go.security.TaintedPath
7+
import codeql.dataflow.test.ProvenancePathGraph
8+
import semmle.go.dataflow.ExternalFlow
9+
import ShowProvenance<interpretModelForTest/2, TaintedPath::Flow::PathNode, TaintedPath::Flow::PathGraph>
10+
11+
from TaintedPath::Flow::PathNode source, TaintedPath::Flow::PathNode sink
12+
where TaintedPath::Flow::flowPath(source, sink)
13+
select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
14+
"user-provided value"

‎go/ql/test/query-tests/Security/CWE-022/TaintedPath.qlref‎

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

‎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 |

0 commit comments

Comments
 (0)