Skip to content

Commit

Permalink
Revert "Merge pull request #18235 from owen-mc/go/varargs-out-param"
Browse files Browse the repository at this point in the history
This reverts commit 4f8645b, reversing
changes made to 22aaf74.
  • Loading branch information
dbartol committed Jan 7, 2025
1 parent 3dcf49c commit 1323b3f
Show file tree
Hide file tree
Showing 26 changed files with 9 additions and 350 deletions.

This file was deleted.

5 changes: 0 additions & 5 deletions go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ predicate containerReadStep(Node node1, Node node2, Content c) {
(
node2.(Read).readsElement(node1, _)
or
exists(ImplicitVarargsSlice ivs |
node1.(PostUpdateNode).getPreUpdateNode() = ivs and
node2.(PostUpdateNode).getPreUpdateNode() = ivs.getCallNode().getAnImplicitVarargsArgument()
)
or
node2.(RangeElementNode).getBase() = node1
or
// To model data flow from array elements of the base of a `SliceNode` to
Expand Down
3 changes: 0 additions & 3 deletions go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll
Original file line number Diff line number Diff line change
Expand Up @@ -845,9 +845,6 @@ module Public {
or
preupd = getAWrittenNode()
or
preupd instanceof ImplicitVarargsSlice and
mutableType(preupd.(ImplicitVarargsSlice).getType().(SliceType).getElementType())
or
preupd = any(ArgumentNode arg).getACorrespondingSyntacticArgument() and
mutableType(preupd.getType())
) and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import semmle.go.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
import utils.test.InlineFlowTest

module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { sourceNode(source, "qltest") }
predicate isSource(DataFlow::Node src) { sourceNode(src, "qltest") }

predicate isSink(DataFlow::Node sink) { sinkNode(sink, "qltest") }
predicate isSink(DataFlow::Node src) { sinkNode(src, "qltest") }
}

import ValueFlowTest<Config>
12 changes: 1 addition & 11 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ func source() string {
return "untrusted data"
}

func sink(any) {
func sink(string) {
}

type A struct {
Expand All @@ -19,10 +19,6 @@ func functionWithVarArgsParameter(s ...string) string {
return s[1]
}

func functionWithVarArgsOutParameter(in string, out ...*string) {
*out[0] = in
}

func functionWithSliceOfStructsParameter(s []A) string {
return s[1].f
}
Expand All @@ -42,12 +38,6 @@ func main() {
sink(functionWithVarArgsParameter(sSlice...)) // $ hasValueFlow="call to functionWithVarArgsParameter"
sink(functionWithVarArgsParameter(s0, s1)) // $ hasValueFlow="call to functionWithVarArgsParameter"

var out1 *string
var out2 *string
functionWithVarArgsOutParameter(source(), out1, out2)
sink(out1) // $ MISSING: hasValueFlow="out1"
sink(out2) // $ MISSING: hasValueFlow="out2"

sliceOfStructs := []A{{f: source()}}
sink(sliceOfStructs[0].f) // $ hasValueFlow="selection of f"

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ class SummaryModelTest extends DataFlow::FunctionModel {
this.hasQualifiedName("github.com/nonexistent/test", "FunctionWithVarArgsParameter") and
(inp.isParameter(_) and outp.isResult())
or
this.hasQualifiedName("github.com/nonexistent/test", "FunctionWithVarArgsOutParameter") and
(inp.isParameter(0) and outp.isParameter(any(int i | i >= 1)))
or
this.hasQualifiedName("github.com/nonexistent/test", "FunctionWithSliceOfStructsParameter") and
(inp.isParameter(0) and outp.isResult())
or
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module semmle.go.Packages

go 1.23
go 1.17

require github.com/nonexistent/test v0.0.0-20200203000000-0000000000000
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ func source() string {
return "untrusted data"
}

func sink(any) {
func sink(string) {
}

func main() {
Expand All @@ -21,17 +21,10 @@ func main() {
s0 := ""
s1 := source()
sSlice := []string{s0, s1}
sink(test.FunctionWithParameter(sSlice[1])) // $ hasValueFlow="call to FunctionWithParameter"
sink(test.FunctionWithSliceParameter(sSlice)) // $ hasTaintFlow="call to FunctionWithSliceParameter" MISSING: hasValueFlow="call to FunctionWithSliceParameter"
sink(test.FunctionWithVarArgsParameter(sSlice...)) // $ hasTaintFlow="call to FunctionWithVarArgsParameter" MISSING: hasValueFlow="call to FunctionWithVarArgsParameter"
randomFunctionWithMoreThanOneParameter(1, 2, 3, 4, 5) // This is needed to make the next line pass, because we need to have seen a call to a function with at least 2 parameters for ParameterInput to exist with index 1.
sink(test.FunctionWithVarArgsParameter(s0, s1)) // $ hasValueFlow="call to FunctionWithVarArgsParameter"

var out1 *string
var out2 *string
test.FunctionWithVarArgsOutParameter(source(), out1, out2)
sink(out1) // $ hasValueFlow="out1"
sink(out2) // $ hasValueFlow="out2"
sink(test.FunctionWithParameter(sSlice[1])) // $ hasValueFlow="call to FunctionWithParameter"
sink(test.FunctionWithSliceParameter(sSlice)) // $ hasTaintFlow="call to FunctionWithSliceParameter" MISSING: hasValueFlow="call to FunctionWithSliceParameter"
sink(test.FunctionWithVarArgsParameter(sSlice...)) // $ hasTaintFlow="call to FunctionWithVarArgsParameter" MISSING: hasValueFlow="call to FunctionWithVarArgsParameter"
sink(test.FunctionWithVarArgsParameter(s0, s1)) // $ MISSING: hasValueFlow="call to FunctionWithVarArgsParameter"

sliceOfStructs := []test.A{{Field: source()}}
sink(sliceOfStructs[0].Field) // $ hasValueFlow="selection of Field"
Expand All @@ -44,6 +37,3 @@ func main() {
sink(test.FunctionWithVarArgsOfStructsParameter(aSlice...)) // $ MISSING: hasValueFlow="call to FunctionWithVarArgsOfStructsParameter"
sink(test.FunctionWithVarArgsOfStructsParameter(a0, a1)) // $ MISSING: hasValueFlow="call to FunctionWithVarArgsOfStructsParameter"
}

func randomFunctionWithMoreThanOneParameter(i1, i2, i3, i4, i5 int) {
}
Binary file not shown.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ edges
| test.go:148:16:148:23 | &... | test.go:149:13:149:39 | type conversion | provenance | |
| test.go:152:15:152:24 | &... | test.go:153:13:153:47 | type conversion | provenance | |
| test.go:156:18:156:30 | &... | test.go:157:13:157:38 | type conversion | provenance | |
| test.go:160:2:160:23 | []type{args} [array] | test.go:160:14:160:22 | &... | provenance | |
| test.go:160:14:160:22 | &... | test.go:160:2:160:23 | []type{args} [array] | provenance | |
| test.go:160:14:160:22 | &... | test.go:161:13:161:28 | type conversion | provenance | |
| test.go:164:2:164:25 | []type{args} [array] | test.go:164:15:164:24 | &... | provenance | |
| test.go:164:15:164:24 | &... | test.go:164:2:164:25 | []type{args} [array] | provenance | |
| test.go:164:15:164:24 | &... | test.go:165:13:165:32 | type conversion | provenance | |
nodes
| test.go:80:13:80:16 | &... | semmle.label | &... |
Expand Down Expand Up @@ -80,10 +76,8 @@ nodes
| test.go:153:13:153:47 | type conversion | semmle.label | type conversion |
| test.go:156:18:156:30 | &... | semmle.label | &... |
| test.go:157:13:157:38 | type conversion | semmle.label | type conversion |
| test.go:160:2:160:23 | []type{args} [array] | semmle.label | []type{args} [array] |
| test.go:160:14:160:22 | &... | semmle.label | &... |
| test.go:161:13:161:28 | type conversion | semmle.label | type conversion |
| test.go:164:2:164:25 | []type{args} [array] | semmle.label | []type{args} [array] |
| test.go:164:15:164:24 | &... | semmle.label | &... |
| test.go:165:13:165:32 | type conversion | semmle.label | type conversion |
subpaths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
edges
| StoredCommand.go:11:2:11:27 | ... := ...[0] | StoredCommand.go:13:2:13:5 | rows | provenance | |
| StoredCommand.go:13:2:13:5 | rows | StoredCommand.go:13:12:13:19 | &... | provenance | FunctionModel |
| StoredCommand.go:13:2:13:20 | []type{args} [array] | StoredCommand.go:13:12:13:19 | &... | provenance | |
| StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:13:2:13:20 | []type{args} [array] | provenance | |
| StoredCommand.go:13:12:13:19 | &... | StoredCommand.go:14:22:14:28 | cmdName | provenance | Sink:MaD:1 |
models
| 1 | Sink: os/exec; ; false; Command; ; ; Argument[0]; command-injection; manual |
nodes
| StoredCommand.go:11:2:11:27 | ... := ...[0] | semmle.label | ... := ...[0] |
| StoredCommand.go:13:2:13:5 | rows | semmle.label | rows |
| StoredCommand.go:13:2:13:20 | []type{args} [array] | semmle.label | []type{args} [array] |
| StoredCommand.go:13:12:13:19 | &... | semmle.label | &... |
| StoredCommand.go:14:22:14:28 | cmdName | semmle.label | cmdName |
subpaths
Loading

0 comments on commit 1323b3f

Please sign in to comment.