Skip to content
This repository was archived by the owner on Jan 5, 2023. It is now read-only.

Commit c2a26f8

Browse files
author
Max Schaefer
committed
Don't allow varargs as function outputs.
In a call of the form `f(xs...)`, when we say that `f` taints its 0th argument its ambiguous whether that means that it taints the slice `xs` or its 0th element `xs[0]`. In practice, it's usually the latter, but we have no way of expressing that using our current `FunctionOutput` implementation.
1 parent bdfd1d1 commit c2a26f8

File tree

5 files changed

+23
-1
lines changed

5 files changed

+23
-1
lines changed

ql/src/semmle/go/dataflow/FunctionInputsAndOutputs.qll

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,12 @@ private class OutReceiver extends FunctionOutput, TOutReceiver {
242242
override string toString() { result = "receiver" }
243243
}
244244

245-
/** A parameter of a function, viewed as an output. */
245+
/**
246+
* A parameter of a function, viewed as an output.
247+
*
248+
* Note that slices passed to varargs parameters using `...` are not included, since in this
249+
* case it is ambiguous whether the output should be the slice itself or one of its elements.
250+
*/
246251
private class OutParameter extends FunctionOutput, TOutParameter {
247252
int index;
248253

@@ -259,6 +264,8 @@ private class OutParameter extends FunctionOutput, TOutParameter {
259264
override DataFlow::Node getExitNode(DataFlow::CallNode c) {
260265
exists(DataFlow::Node arg |
261266
arg = getArgument(c, index) and
267+
not (c.hasEllipsis() and index = c.getNumArgument() - 1)
268+
|
262269
result.(DataFlow::PostUpdateNode).getPreUpdateNode() = arg
263270
)
264271
}

ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getEntryNode.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
| parameter 0 | main.go:57:2:57:27 | call to Printf | main.go:57:13:57:20 | "%d, %d" |
55
| parameter 0 | reset.go:12:2:12:21 | call to Reset | reset.go:12:15:12:20 | source |
66
| parameter 0 | tst.go:10:2:10:29 | call to ReadFrom | tst.go:10:23:10:28 | reader |
7+
| parameter 0 | tst.go:16:2:16:12 | call to test5 | tst.go:16:8:16:8 | x |
8+
| parameter 0 | tst.go:18:2:18:12 | call to test5 | tst.go:18:8:18:8 | s |
79
| parameter 1 | main.go:51:2:51:14 | call to op | main.go:51:10:51:10 | 1 |
810
| parameter 1 | main.go:53:2:53:22 | call to op2 | main.go:53:11:53:11 | 2 |
911
| parameter 1 | main.go:55:2:55:27 | call to Printf | main.go:55:23:55:23 | x |
1012
| parameter 1 | main.go:57:2:57:27 | call to Printf | main.go:57:23:57:23 | x |
13+
| parameter 1 | tst.go:16:2:16:12 | call to test5 | tst.go:16:11:16:11 | y |
1114
| parameter 2 | main.go:51:2:51:14 | call to op | main.go:51:13:51:13 | 1 |
1215
| parameter 2 | main.go:53:2:53:22 | call to op2 | main.go:53:14:53:21 | call to bump |
1316
| parameter 2 | main.go:55:2:55:27 | call to Printf | main.go:55:26:55:26 | y |

ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionInput_getExitNode.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
| parameter 0 | main.go:40:1:48:1 | function declaration | main.go:40:12:40:12 | definition of b |
44
| parameter 0 | reset.go:8:1:16:1 | function declaration | reset.go:8:27:8:27 | definition of r |
55
| parameter 0 | tst.go:8:1:11:1 | function declaration | tst.go:8:12:8:17 | definition of reader |
6+
| parameter 0 | tst.go:15:1:19:1 | function declaration | tst.go:15:12:15:12 | definition of x |
67
| parameter 1 | main.go:5:1:11:1 | function declaration | main.go:5:20:5:20 | definition of x |
78
| parameter 1 | main.go:13:1:20:1 | function declaration | main.go:13:21:13:21 | definition of x |
9+
| parameter 1 | tst.go:15:1:19:1 | function declaration | tst.go:15:15:15:15 | definition of y |
810
| parameter 2 | main.go:5:1:11:1 | function declaration | main.go:5:27:5:27 | definition of y |
911
| parameter 2 | main.go:13:1:20:1 | function declaration | main.go:13:28:13:28 | definition of y |
1012
| receiver | main.go:26:1:29:1 | function declaration | main.go:26:7:26:7 | definition of c |

ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/FunctionOutput_getExitNode.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
| parameter 0 | reset.go:12:2:12:21 | call to Reset | reset.go:9:2:9:7 | definition of source |
22
| parameter 0 | tst.go:10:2:10:29 | call to ReadFrom | tst.go:8:12:8:17 | definition of reader |
3+
| parameter 0 | tst.go:16:2:16:12 | call to test5 | tst.go:15:12:15:12 | definition of x |
4+
| parameter 1 | tst.go:16:2:16:12 | call to test5 | tst.go:15:15:15:15 | definition of y |
35
| receiver | main.go:53:14:53:21 | call to bump | main.go:52:2:52:2 | definition of c |
46
| receiver | reset.go:12:2:12:21 | call to Reset | reset.go:11:6:11:11 | definition of reader |
57
| receiver | tst.go:10:2:10:29 | call to ReadFrom | tst.go:9:2:9:12 | definition of bytesBuffer |

ql/test/library-tests/semmle/go/dataflow/FunctionInputsAndOutputs/tst.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ func test4(reader io.Reader) {
99
bytesBuffer := new(bytes.Buffer)
1010
bytesBuffer.ReadFrom(reader)
1111
}
12+
13+
func test5(xs ...*int) {}
14+
15+
func test6(x, y *int) {
16+
test5(x, y)
17+
s := []*int{x, y}
18+
test5(s...)
19+
}

0 commit comments

Comments
 (0)