Skip to content

Commit 1f01afb

Browse files
owen-mcowen-mc
committed
Fix flow to variable capture
The jump step to a `SsaCaptureVariable` should start at the last use before it, rather than from the previous definition.
1 parent b00e022 commit 1f01afb

File tree

2 files changed

+17
-24
lines changed

2 files changed

+17
-24
lines changed

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,18 @@ predicate jumpStep(Node n1, Node n2) {
120120
n2 = v.getARead()
121121
)
122122
or
123-
exists(SsaDefinition pred, SsaDefinition succ |
124-
succ.(SsaVariableCapture).getSourceVariable() = pred.(SsaExplicitDefinition).getSourceVariable()
125-
|
126-
n1 = ssaNode(pred) and
123+
exists(SsaExplicitDefinition def, SsaVariableCapture succ |
124+
succ.getSourceVariable() = def.getSourceVariable() and
127125
n2 = ssaNode(succ)
126+
|
127+
not exists(def.getAFirstUse()) and n1 = ssaNode(def)
128+
or
129+
exists(IR::Instruction lastUse |
130+
lastUse = getAnAdjacentUse*(def.getAFirstUse()) and
131+
not exists(getAnAdjacentUse(lastUse))
132+
|
133+
[n1, n1.(DataFlow::PostUpdateNode).getPreUpdateNode()] = instructionNode(lastUse)
134+
)
128135
)
129136
or
130137
// If a channel-typed field is referenced exactly once in the context of

‎go/ql/test/library-tests/semmle/go/frameworks/Twirp/RequestForgery.expected‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,42 +3,28 @@
33
| server/main.go:30:38:30:48 | selection of Text | server/main.go:19:56:19:61 | definition of params | server/main.go:30:38:30:48 | selection of Text | The $@ of this request depends on a $@. | server/main.go:30:38:30:48 | selection of Text | URL | server/main.go:19:56:19:61 | definition of params | user-provided value |
44
edges
55
| client/main.go:16:35:16:78 | &... | server/main.go:19:56:19:61 | definition of params | provenance | |
6-
| rpc/notes/service.twirp.go:473:6:473:13 | definition of typedReq | rpc/notes/service.twirp.go:477:44:477:51 | typedReq | provenance | |
7-
| rpc/notes/service.twirp.go:477:44:477:51 | typedReq | server/main.go:19:56:19:61 | definition of params | provenance | |
8-
| rpc/notes/service.twirp.go:493:2:496:2 | capture variable reqContent | rpc/notes/service.twirp.go:495:35:495:44 | reqContent | provenance | |
9-
| rpc/notes/service.twirp.go:495:35:495:44 | reqContent | server/main.go:19:56:19:61 | definition of params | provenance | |
6+
| client/main.go:16:35:16:78 | &... [postupdate] | client/main.go:16:35:16:78 | &... | provenance | |
107
| rpc/notes/service.twirp.go:538:2:538:33 | ... := ...[0] | rpc/notes/service.twirp.go:544:27:544:29 | buf | provenance | |
118
| rpc/notes/service.twirp.go:538:25:538:32 | selection of Body | rpc/notes/service.twirp.go:538:2:538:33 | ... := ...[0] | provenance | Src:MaD:1 MaD:3 |
12-
| rpc/notes/service.twirp.go:543:2:543:11 | definition of reqContent | rpc/notes/service.twirp.go:574:2:577:2 | capture variable reqContent | provenance | |
13-
| rpc/notes/service.twirp.go:544:27:544:29 | buf | rpc/notes/service.twirp.go:543:2:543:11 | definition of reqContent | provenance | MaD:2 |
14-
| rpc/notes/service.twirp.go:554:6:554:13 | definition of typedReq | rpc/notes/service.twirp.go:558:44:558:51 | typedReq | provenance | |
15-
| rpc/notes/service.twirp.go:558:44:558:51 | typedReq | server/main.go:19:56:19:61 | definition of params | provenance | |
9+
| rpc/notes/service.twirp.go:544:27:544:29 | buf | rpc/notes/service.twirp.go:544:32:544:41 | reqContent [postupdate] | provenance | MaD:2 |
10+
| rpc/notes/service.twirp.go:544:32:544:41 | reqContent [postupdate] | rpc/notes/service.twirp.go:574:2:577:2 | capture variable reqContent | provenance | |
1611
| rpc/notes/service.twirp.go:574:2:577:2 | capture variable reqContent | rpc/notes/service.twirp.go:576:35:576:44 | reqContent | provenance | |
1712
| rpc/notes/service.twirp.go:576:35:576:44 | reqContent | server/main.go:19:56:19:61 | definition of params | provenance | |
1813
| server/main.go:19:56:19:61 | definition of params | server/main.go:19:56:19:61 | definition of params [Return] | provenance | |
1914
| server/main.go:19:56:19:61 | definition of params | server/main.go:30:38:30:48 | selection of Text | provenance | |
2015
| server/main.go:19:56:19:61 | definition of params | server/main.go:30:38:30:48 | selection of Text | provenance | |
21-
| server/main.go:19:56:19:61 | definition of params [Return] | client/main.go:16:35:16:78 | &... | provenance | |
22-
| server/main.go:19:56:19:61 | definition of params [Return] | rpc/notes/service.twirp.go:473:6:473:13 | definition of typedReq | provenance | |
23-
| server/main.go:19:56:19:61 | definition of params [Return] | rpc/notes/service.twirp.go:493:2:496:2 | capture variable reqContent | provenance | |
24-
| server/main.go:19:56:19:61 | definition of params [Return] | rpc/notes/service.twirp.go:554:6:554:13 | definition of typedReq | provenance | |
25-
| server/main.go:19:56:19:61 | definition of params [Return] | rpc/notes/service.twirp.go:574:2:577:2 | capture variable reqContent | provenance | |
16+
| server/main.go:19:56:19:61 | definition of params [Return] | client/main.go:16:35:16:78 | &... [postupdate] | provenance | |
2617
models
2718
| 1 | Source: net/http; Request; true; Body; ; ; ; remote; manual |
2819
| 2 | Summary: google.golang.org/protobuf/proto; ; false; Unmarshal; ; ; Argument[0]; Argument[1]; taint; manual |
2920
| 3 | Summary: io; ; false; ReadAll; ; ; Argument[0]; ReturnValue[0]; taint; manual |
3021
nodes
3122
| client/main.go:16:35:16:78 | &... | semmle.label | &... |
32-
| rpc/notes/service.twirp.go:473:6:473:13 | definition of typedReq | semmle.label | definition of typedReq |
33-
| rpc/notes/service.twirp.go:477:44:477:51 | typedReq | semmle.label | typedReq |
34-
| rpc/notes/service.twirp.go:493:2:496:2 | capture variable reqContent | semmle.label | capture variable reqContent |
35-
| rpc/notes/service.twirp.go:495:35:495:44 | reqContent | semmle.label | reqContent |
23+
| client/main.go:16:35:16:78 | &... [postupdate] | semmle.label | &... [postupdate] |
3624
| rpc/notes/service.twirp.go:538:2:538:33 | ... := ...[0] | semmle.label | ... := ...[0] |
3725
| rpc/notes/service.twirp.go:538:25:538:32 | selection of Body | semmle.label | selection of Body |
38-
| rpc/notes/service.twirp.go:543:2:543:11 | definition of reqContent | semmle.label | definition of reqContent |
3926
| rpc/notes/service.twirp.go:544:27:544:29 | buf | semmle.label | buf |
40-
| rpc/notes/service.twirp.go:554:6:554:13 | definition of typedReq | semmle.label | definition of typedReq |
41-
| rpc/notes/service.twirp.go:558:44:558:51 | typedReq | semmle.label | typedReq |
27+
| rpc/notes/service.twirp.go:544:32:544:41 | reqContent [postupdate] | semmle.label | reqContent [postupdate] |
4228
| rpc/notes/service.twirp.go:574:2:577:2 | capture variable reqContent | semmle.label | capture variable reqContent |
4329
| rpc/notes/service.twirp.go:576:35:576:44 | reqContent | semmle.label | reqContent |
4430
| server/main.go:19:56:19:61 | definition of params | semmle.label | definition of params |

0 commit comments

Comments
 (0)