Skip to content

Commit b3a19ef

Browse files
owen-mcowen-mc
authored
Merge pull request github#13461 from owen-mc/go/show-functionmodel-steps-to-path-summaries
Go: show FunctionModel steps in path summaries
2 parents 7c2f26e + 0baf78f commit b3a19ef

File tree

16 files changed

+203
-46
lines changed

16 files changed

+203
-46
lines changed

‎go/ql/lib/change-notes/2023-06-20-function-model-path-nodes.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* When a result of path query flows through a function modeled using `DataFlow::FunctionModel` or `TaintTracking::FunctionModel`, the path now includes nodes corresponding to the input and output to the function. This brings it in line with functions modeled using Models-as-Data.

‎go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,11 @@ class CastNode extends ExprNode {
232232
* Holds if `n` should never be skipped over in the `PathGraph` and in path
233233
* explanations.
234234
*/
235-
predicate neverSkipInPathGraph(Node n) { none() }
235+
predicate neverSkipInPathGraph(Node n) {
236+
exists(DataFlow::FunctionModel fm | fm.getAnInputNode(_) = n or fm.getAnOutputNode(_) = n)
237+
or
238+
exists(TaintTracking::FunctionModel fm | fm.getAnInputNode(_) = n or fm.getAnOutputNode(_) = n)
239+
}
236240

237241
class DataFlowExpr = Expr;
238242

‎go/ql/test/experimental/CWE-134/DsnInjection.expected

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
edges
2-
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:50:29:50:33 | dbDSN |
2+
| Dsn.go:47:10:47:30 | call to FormValue | Dsn.go:49:102:49:105 | name |
3+
| Dsn.go:49:11:49:106 | call to Sprintf | Dsn.go:50:29:50:33 | dbDSN |
4+
| Dsn.go:49:102:49:105 | name | Dsn.go:49:11:49:106 | call to Sprintf |
35
nodes
46
| Dsn.go:47:10:47:30 | call to FormValue | semmle.label | call to FormValue |
7+
| Dsn.go:49:11:49:106 | call to Sprintf | semmle.label | call to Sprintf |
8+
| Dsn.go:49:102:49:105 | name | semmle.label | name |
59
| Dsn.go:50:29:50:33 | dbDSN | semmle.label | dbDSN |
610
subpaths
711
#select

‎go/ql/test/experimental/CWE-134/DsnInjectionLocal.expected

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,34 @@
11
edges
2-
| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:29:29:29:33 | dbDSN |
2+
| Dsn.go:26:11:26:17 | selection of Args | Dsn.go:28:102:28:109 | index expression |
3+
| Dsn.go:28:11:28:110 | call to Sprintf | Dsn.go:29:29:29:33 | dbDSN |
4+
| Dsn.go:28:102:28:109 | index expression | Dsn.go:28:11:28:110 | call to Sprintf |
35
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:63:9:63:11 | cfg [pointer] |
46
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | Dsn.go:67:102:67:104 | cfg [pointer] |
57
| Dsn.go:63:9:63:11 | cfg [pointer] | Dsn.go:63:9:63:11 | implicit dereference |
68
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:62:2:62:4 | definition of cfg [pointer] |
79
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
8-
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
9-
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:9:63:11 | implicit dereference |
10-
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:68:29:68:33 | dbDSN |
10+
| Dsn.go:63:9:63:11 | implicit dereference | Dsn.go:67:102:67:108 | selection of dsn |
11+
| Dsn.go:63:19:63:25 | selection of Args | Dsn.go:63:19:63:29 | slice expression |
12+
| Dsn.go:63:19:63:29 | slice expression | Dsn.go:63:9:63:11 | implicit dereference |
13+
| Dsn.go:67:11:67:109 | call to Sprintf | Dsn.go:68:29:68:33 | dbDSN |
1114
| Dsn.go:67:102:67:104 | cfg [pointer] | Dsn.go:67:102:67:104 | implicit dereference |
1215
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:63:9:63:11 | implicit dereference |
13-
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:68:29:68:33 | dbDSN |
16+
| Dsn.go:67:102:67:104 | implicit dereference | Dsn.go:67:102:67:108 | selection of dsn |
17+
| Dsn.go:67:102:67:108 | selection of dsn | Dsn.go:67:11:67:109 | call to Sprintf |
1418
nodes
1519
| Dsn.go:26:11:26:17 | selection of Args | semmle.label | selection of Args |
20+
| Dsn.go:28:11:28:110 | call to Sprintf | semmle.label | call to Sprintf |
21+
| Dsn.go:28:102:28:109 | index expression | semmle.label | index expression |
1622
| Dsn.go:29:29:29:33 | dbDSN | semmle.label | dbDSN |
1723
| Dsn.go:62:2:62:4 | definition of cfg [pointer] | semmle.label | definition of cfg [pointer] |
1824
| Dsn.go:63:9:63:11 | cfg [pointer] | semmle.label | cfg [pointer] |
1925
| Dsn.go:63:9:63:11 | implicit dereference | semmle.label | implicit dereference |
2026
| Dsn.go:63:19:63:25 | selection of Args | semmle.label | selection of Args |
27+
| Dsn.go:63:19:63:29 | slice expression | semmle.label | slice expression |
28+
| Dsn.go:67:11:67:109 | call to Sprintf | semmle.label | call to Sprintf |
2129
| Dsn.go:67:102:67:104 | cfg [pointer] | semmle.label | cfg [pointer] |
2230
| Dsn.go:67:102:67:104 | implicit dereference | semmle.label | implicit dereference |
31+
| Dsn.go:67:102:67:108 | selection of dsn | semmle.label | selection of dsn |
2332
| Dsn.go:68:29:68:33 | dbDSN | semmle.label | dbDSN |
2433
subpaths
2534
#select

‎go/ql/test/experimental/CWE-918/SSRF.expected

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,23 @@ edges
44
| builtin.go:97:21:97:31 | call to Referer | builtin.go:101:36:101:49 | untrustedInput |
55
| builtin.go:111:21:111:31 | call to Referer | builtin.go:114:15:114:28 | untrustedInput |
66
| builtin.go:129:21:129:31 | call to Referer | builtin.go:132:38:132:51 | untrustedInput |
7-
| new-tests.go:26:26:26:30 | &... | new-tests.go:31:11:31:57 | call to Sprintf |
8-
| new-tests.go:26:26:26:30 | &... | new-tests.go:32:11:32:57 | call to Sprintf |
9-
| new-tests.go:26:26:26:30 | &... | new-tests.go:35:12:35:58 | call to Sprintf |
7+
| new-tests.go:26:26:26:30 | &... | new-tests.go:31:48:31:56 | selection of word |
8+
| new-tests.go:26:26:26:30 | &... | new-tests.go:32:48:32:56 | selection of safe |
9+
| new-tests.go:26:26:26:30 | &... | new-tests.go:35:49:35:57 | selection of word |
10+
| new-tests.go:31:48:31:56 | selection of word | new-tests.go:31:11:31:57 | call to Sprintf |
11+
| new-tests.go:32:48:32:56 | selection of safe | new-tests.go:32:11:32:57 | call to Sprintf |
12+
| new-tests.go:35:49:35:57 | selection of word | new-tests.go:35:12:35:58 | call to Sprintf |
1013
| new-tests.go:39:18:39:30 | call to Param | new-tests.go:47:11:47:46 | ...+... |
1114
| new-tests.go:49:18:49:30 | call to Query | new-tests.go:50:11:50:46 | ...+... |
1215
| new-tests.go:62:2:62:39 | ... := ...[0] | new-tests.go:63:17:63:23 | reqBody |
1316
| new-tests.go:62:31:62:38 | selection of Body | new-tests.go:62:2:62:39 | ... := ...[0] |
1417
| new-tests.go:63:17:63:23 | reqBody | new-tests.go:63:26:63:30 | &... |
15-
| new-tests.go:63:26:63:30 | &... | new-tests.go:68:11:68:57 | call to Sprintf |
16-
| new-tests.go:63:26:63:30 | &... | new-tests.go:69:11:69:57 | call to Sprintf |
17-
| new-tests.go:63:26:63:30 | &... | new-tests.go:74:12:74:58 | call to Sprintf |
18+
| new-tests.go:63:26:63:30 | &... | new-tests.go:68:48:68:56 | selection of word |
19+
| new-tests.go:63:26:63:30 | &... | new-tests.go:69:48:69:56 | selection of safe |
20+
| new-tests.go:63:26:63:30 | &... | new-tests.go:74:49:74:57 | selection of word |
21+
| new-tests.go:68:48:68:56 | selection of word | new-tests.go:68:11:68:57 | call to Sprintf |
22+
| new-tests.go:69:48:69:56 | selection of safe | new-tests.go:69:11:69:57 | call to Sprintf |
23+
| new-tests.go:74:49:74:57 | selection of word | new-tests.go:74:12:74:58 | call to Sprintf |
1824
| new-tests.go:78:18:78:24 | selection of URL | new-tests.go:78:18:78:32 | call to Query |
1925
| new-tests.go:78:18:78:32 | call to Query | new-tests.go:78:18:78:46 | call to Get |
2026
| new-tests.go:78:18:78:46 | call to Get | new-tests.go:79:11:79:46 | ...+... |
@@ -36,8 +42,11 @@ nodes
3642
| builtin.go:132:38:132:51 | untrustedInput | semmle.label | untrustedInput |
3743
| new-tests.go:26:26:26:30 | &... | semmle.label | &... |
3844
| new-tests.go:31:11:31:57 | call to Sprintf | semmle.label | call to Sprintf |
45+
| new-tests.go:31:48:31:56 | selection of word | semmle.label | selection of word |
3946
| new-tests.go:32:11:32:57 | call to Sprintf | semmle.label | call to Sprintf |
47+
| new-tests.go:32:48:32:56 | selection of safe | semmle.label | selection of safe |
4048
| new-tests.go:35:12:35:58 | call to Sprintf | semmle.label | call to Sprintf |
49+
| new-tests.go:35:49:35:57 | selection of word | semmle.label | selection of word |
4150
| new-tests.go:39:18:39:30 | call to Param | semmle.label | call to Param |
4251
| new-tests.go:47:11:47:46 | ...+... | semmle.label | ...+... |
4352
| new-tests.go:49:18:49:30 | call to Query | semmle.label | call to Query |
@@ -47,8 +56,11 @@ nodes
4756
| new-tests.go:63:17:63:23 | reqBody | semmle.label | reqBody |
4857
| new-tests.go:63:26:63:30 | &... | semmle.label | &... |
4958
| new-tests.go:68:11:68:57 | call to Sprintf | semmle.label | call to Sprintf |
59+
| new-tests.go:68:48:68:56 | selection of word | semmle.label | selection of word |
5060
| new-tests.go:69:11:69:57 | call to Sprintf | semmle.label | call to Sprintf |
61+
| new-tests.go:69:48:69:56 | selection of safe | semmle.label | selection of safe |
5162
| new-tests.go:74:12:74:58 | call to Sprintf | semmle.label | call to Sprintf |
63+
| new-tests.go:74:49:74:57 | selection of word | semmle.label | selection of word |
5264
| new-tests.go:78:18:78:24 | selection of URL | semmle.label | selection of URL |
5365
| new-tests.go:78:18:78:32 | call to Query | semmle.label | call to Query |
5466
| new-tests.go:78:18:78:46 | call to Get | semmle.label | call to Get |

‎go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.expected

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ edges
4747
| test.go:240:15:240:36 | call to GetString | test.go:243:21:243:29 | untrusted |
4848
| test.go:253:23:253:44 | call to GetCookie | test.go:253:16:253:45 | type conversion |
4949
| test.go:264:62:264:83 | call to GetCookie | test.go:264:55:264:84 | type conversion |
50-
| test.go:269:2:269:40 | ... := ...[0] | test.go:277:21:277:61 | call to GetDisplayString |
50+
| test.go:269:2:269:40 | ... := ...[0] | test.go:277:44:277:60 | selection of Filename |
5151
| test.go:269:2:269:40 | ... := ...[0] | test.go:278:38:278:49 | genericFiles |
5252
| test.go:269:2:269:40 | ... := ...[0] | test.go:279:37:279:48 | genericFiles |
5353
| test.go:269:2:269:40 | ... := ...[0] | test.go:285:4:285:15 | genericFiles |
@@ -61,6 +61,7 @@ edges
6161
| test.go:269:2:269:40 | ... := ...[0] | test.go:295:39:295:50 | genericFiles |
6262
| test.go:269:2:269:40 | ... := ...[0] | test.go:296:40:296:51 | genericFiles |
6363
| test.go:269:2:269:40 | ... := ...[0] | test.go:297:39:297:50 | genericFiles |
64+
| test.go:277:44:277:60 | selection of Filename | test.go:277:21:277:61 | call to GetDisplayString |
6465
| test.go:278:21:278:53 | call to SliceChunk | test.go:278:21:278:92 | selection of Filename |
6566
| test.go:278:38:278:49 | genericFiles | test.go:278:21:278:53 | call to SliceChunk |
6667
| test.go:279:21:279:60 | call to SliceDiff | test.go:279:21:279:96 | selection of Filename |
@@ -177,6 +178,7 @@ nodes
177178
| test.go:264:62:264:83 | call to GetCookie | semmle.label | call to GetCookie |
178179
| test.go:269:2:269:40 | ... := ...[0] | semmle.label | ... := ...[0] |
179180
| test.go:277:21:277:61 | call to GetDisplayString | semmle.label | call to GetDisplayString |
181+
| test.go:277:44:277:60 | selection of Filename | semmle.label | selection of Filename |
180182
| test.go:278:21:278:53 | call to SliceChunk | semmle.label | call to SliceChunk |
181183
| test.go:278:21:278:92 | selection of Filename | semmle.label | selection of Filename |
182184
| test.go:278:38:278:49 | genericFiles | semmle.label | genericFiles |

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
edges
22
| TaintedPath.go:13:18:13:22 | selection of URL | TaintedPath.go:13:18:13:30 | call to Query |
33
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:16:29:16:40 | tainted_path |
4-
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:20:28:20:69 | call to Join |
4+
| TaintedPath.go:13:18:13:30 | call to Query | TaintedPath.go:20:57:20:68 | tainted_path |
5+
| TaintedPath.go:20:57:20:68 | tainted_path | TaintedPath.go:20:28:20:69 | call to Join |
56
| tst.go:14:2:14:39 | ... := ...[1] | tst.go:17:41:17:56 | selection of Filename |
67
nodes
78
| TaintedPath.go:13:18:13:22 | selection of URL | semmle.label | selection of URL |
89
| TaintedPath.go:13:18:13:30 | call to Query | semmle.label | call to Query |
910
| TaintedPath.go:16:29:16:40 | tainted_path | semmle.label | tainted_path |
1011
| TaintedPath.go:20:28:20:69 | call to Join | semmle.label | call to Join |
12+
| TaintedPath.go:20:57:20:68 | tainted_path | semmle.label | tainted_path |
1113
| tst.go:14:2:14:39 | ... := ...[1] | semmle.label | ... := ...[1] |
1214
| tst.go:17:41:17:56 | selection of Filename | semmle.label | selection of Filename |
1315
subpaths

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
edges
2-
| UnsafeUnzipSymlinkGood.go:52:24:52:32 | definition of candidate | UnsafeUnzipSymlinkGood.go:61:31:61:62 | call to Join |
2+
| UnsafeUnzipSymlinkGood.go:52:24:52:32 | definition of candidate | UnsafeUnzipSymlinkGood.go:61:53:61:61 | candidate |
3+
| UnsafeUnzipSymlinkGood.go:61:53:61:61 | candidate | UnsafeUnzipSymlinkGood.go:61:31:61:62 | call to Join |
34
| UnsafeUnzipSymlinkGood.go:72:3:72:25 | ... := ...[0] | UnsafeUnzipSymlinkGood.go:76:24:76:38 | selection of Linkname |
45
| UnsafeUnzipSymlinkGood.go:72:3:72:25 | ... := ...[0] | UnsafeUnzipSymlinkGood.go:76:70:76:80 | selection of Name |
56
| UnsafeUnzipSymlinkGood.go:76:24:76:38 | selection of Linkname | UnsafeUnzipSymlinkGood.go:52:24:52:32 | definition of candidate |
@@ -13,6 +14,7 @@ edges
1314
nodes
1415
| UnsafeUnzipSymlinkGood.go:52:24:52:32 | definition of candidate | semmle.label | definition of candidate |
1516
| UnsafeUnzipSymlinkGood.go:61:31:61:62 | call to Join | semmle.label | call to Join |
17+
| UnsafeUnzipSymlinkGood.go:61:53:61:61 | candidate | semmle.label | candidate |
1618
| UnsafeUnzipSymlinkGood.go:72:3:72:25 | ... := ...[0] | semmle.label | ... := ...[0] |
1719
| UnsafeUnzipSymlinkGood.go:76:24:76:38 | selection of Linkname | semmle.label | selection of Linkname |
1820
| UnsafeUnzipSymlinkGood.go:76:70:76:80 | selection of Name | semmle.label | selection of Name |

0 commit comments

Comments
 (0)