Skip to content

Revert two Go PRs #18434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dbartol merged 2 commits into from
Jan 7, 2025
Merged

Revert two Go PRs #18434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3dcf49c
Revert "Merge pull request #18275 from owen-mc/go/mad/variadic-params…
Jan 7, 2025
1323b3f
Revert "Merge pull request #18235 from owen-mc/go/varargs-out-param"
Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions go/ql/lib/change-notes/2024-12-06-improve-flow-out-of-variadic-parameter.md
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions go/ql/lib/change-notes/2024-12-12-variadic-parameter-sources.md
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
View file
Open in desktop
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
View file
Open in desktop
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
10 changes: 0 additions & 10 deletions go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,13 +458,3 @@ class ContentApprox = Unit;
/** Gets an approximated value for content `c`. */
pragma[inline]
ContentApprox getContentApprox(Content c) { any() }

/**
* Holds if the the content `c` is a container.
*/
predicate containerContent(ContentSet c) {
c instanceof ArrayContent or
c instanceof CollectionContent or
c instanceof MapKeyContent or
c instanceof MapValueContent
}
38 changes: 8 additions & 30 deletions go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,11 @@ predicate localExprTaint(Expr src, Expr sink) {
* Holds if taint can flow in one local step from `src` to `sink`.
*/
predicate localTaintStep(DataFlow::Node src, DataFlow::Node sink) {
DataFlow::localFlowStep(src, sink)
or
localAdditionalTaintStep(src, sink, _)
or
DataFlow::localFlowStep(src, sink) or
localAdditionalTaintStep(src, sink, _) or
// Simple flow through library code is included in the exposed local
// step relation, even though flow is technically inter-procedural
FlowSummaryImpl::Private::Steps::summaryThroughStepTaint(src, sink, _)
or
// Treat container flow as taint for the local taint flow relation
exists(DataFlow::Content c | DataFlowPrivate::containerContent(c) |
DataFlowPrivate::readStep(src, c, sink) or
DataFlowPrivate::storeStep(src, c, sink) or
FlowSummaryImpl::Private::Steps::summaryGetterStep(src, c, sink, _) or
FlowSummaryImpl::Private::Steps::summarySetterStep(src, c, sink, _)
)
}

private Type getElementType(Type containerType) {
Expand Down Expand Up @@ -98,18 +88,12 @@ class AdditionalTaintStep extends Unit {
*/
predicate localAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ, string model) {
(
referenceStep(pred, succ)
or
elementWriteStep(pred, succ)
or
fieldReadStep(pred, succ)
or
elementStep(pred, succ)
or
tupleStep(pred, succ)
or
stringConcatStep(pred, succ)
or
referenceStep(pred, succ) or
elementWriteStep(pred, succ) or
fieldReadStep(pred, succ) or
elementStep(pred, succ) or
tupleStep(pred, succ) or
stringConcatStep(pred, succ) or
sliceStep(pred, succ)
) and
model = ""
Expand Down Expand Up @@ -179,12 +163,6 @@ predicate elementStep(DataFlow::Node pred, DataFlow::Node succ) {
// only step into the value, not the index
succ.asInstruction() = IR::extractTupleElement(nextEntry, 1)
)
or
exists(DataFlow::ImplicitVarargsSlice ivs |
pred.(DataFlow::PostUpdateNode).getPreUpdateNode() = ivs and
succ.(DataFlow::PostUpdateNode).getPreUpdateNode() =
ivs.getCallNode().getAnImplicitVarargsArgument()
)
}

/** Holds if taint flows from `pred` to `succ` via an extract tuple operation. */
Expand Down
4 changes: 2 additions & 2 deletions go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ql
View file
Open in desktop
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>
28 changes: 0 additions & 28 deletions go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,24 @@
| main.go:38:19:38:19 | 3 | main.go:38:7:38:20 | slice literal |
| main.go:39:8:39:25 | []type{args} | main.go:39:8:39:25 | call to append |
| main.go:39:15:39:15 | s | main.go:39:8:39:25 | call to append |
| main.go:39:18:39:18 | 4 | main.go:39:8:39:25 | []type{args} |
| main.go:39:21:39:21 | 5 | main.go:39:8:39:25 | []type{args} |
| main.go:39:24:39:24 | 6 | main.go:39:8:39:25 | []type{args} |
| main.go:40:15:40:15 | s | main.go:40:8:40:23 | call to append |
| main.go:40:18:40:19 | s1 | main.go:40:8:40:23 | call to append |
| main.go:42:10:42:11 | s4 | main.go:38:2:38:2 | definition of s |
| main.go:47:20:47:21 | next key-value pair in range | main.go:47:2:50:2 | range statement[0] |
| main.go:47:20:47:21 | next key-value pair in range | main.go:47:2:50:2 | range statement[1] |
| main.go:47:20:47:21 | xs | main.go:47:2:50:2 | range statement[1] |
| main.go:56:8:56:11 | true | main.go:56:2:56:3 | ch |
| main.go:57:4:57:5 | ch | main.go:57:2:57:5 | <-... |
| strings.go:9:24:9:24 | s | strings.go:9:8:9:38 | call to Replace |
| strings.go:9:32:9:34 | "_" | strings.go:9:8:9:38 | call to Replace |
| strings.go:10:27:10:27 | s | strings.go:10:8:10:42 | call to ReplaceAll |
| strings.go:10:35:10:41 | "&amp;" | strings.go:10:8:10:42 | call to ReplaceAll |
| strings.go:11:9:11:26 | []type{args} | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:9:11:26 | call to Sprint | strings.go:11:9:11:50 | ...+... |
| strings.go:11:9:11:50 | ...+... | strings.go:11:9:11:69 | ...+... |
| strings.go:11:20:11:21 | s2 | strings.go:11:9:11:26 | []type{args} |
| strings.go:11:20:11:21 | s2 | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:24:11:25 | s3 | strings.go:11:9:11:26 | []type{args} |
| strings.go:11:24:11:25 | s3 | strings.go:11:9:11:26 | call to Sprint |
| strings.go:11:30:11:50 | []type{args} | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:30:11:50 | call to Sprintf | strings.go:11:9:11:50 | ...+... |
| strings.go:11:42:11:45 | "%q" | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:48:11:49 | s2 | strings.go:11:30:11:50 | []type{args} |
| strings.go:11:48:11:49 | s2 | strings.go:11:30:11:50 | call to Sprintf |
| strings.go:11:54:11:69 | []type{args} | strings.go:11:54:11:69 | call to Sprintln |
| strings.go:11:54:11:69 | call to Sprintln | strings.go:11:9:11:69 | ...+... |
| strings.go:11:67:11:68 | s3 | strings.go:11:54:11:69 | []type{args} |
| strings.go:11:67:11:68 | s3 | strings.go:11:54:11:69 | call to Sprintln |
| url.go:12:14:12:48 | call to PathUnescape | url.go:12:3:12:48 | ... = ...[0] |
| url.go:12:14:12:48 | call to PathUnescape | url.go:12:3:12:48 | ... = ...[1] |
Expand All @@ -51,25 +39,17 @@
| url.go:27:9:27:30 | call to ParseRequestURI | url.go:27:2:27:30 | ... = ...[1] |
| url.go:27:29:27:29 | s | url.go:27:2:27:30 | ... = ...[0] |
| url.go:28:14:28:14 | u | url.go:28:14:28:28 | call to EscapedPath |
| url.go:28:14:28:28 | call to EscapedPath | url.go:28:2:28:29 | []type{args} |
| url.go:29:14:29:14 | u | url.go:29:14:29:25 | call to Hostname |
| url.go:29:14:29:25 | call to Hostname | url.go:29:2:29:26 | []type{args} |
| url.go:30:11:30:11 | u | url.go:30:2:30:27 | ... := ...[0] |
| url.go:30:11:30:27 | call to MarshalBinary | url.go:30:2:30:27 | ... := ...[0] |
| url.go:30:11:30:27 | call to MarshalBinary | url.go:30:2:30:27 | ... := ...[1] |
| url.go:31:2:31:16 | []type{args} | url.go:30:2:30:3 | definition of bs |
| url.go:31:14:31:15 | bs | url.go:31:2:31:16 | []type{args} |
| url.go:32:9:32:9 | u | url.go:32:2:32:23 | ... = ...[0] |
| url.go:32:9:32:23 | call to Parse | url.go:32:2:32:23 | ... = ...[0] |
| url.go:32:9:32:23 | call to Parse | url.go:32:2:32:23 | ... = ...[1] |
| url.go:32:17:32:22 | "/foo" | url.go:32:2:32:23 | ... = ...[0] |
| url.go:33:14:33:14 | u | url.go:33:14:33:21 | call to Port |
| url.go:33:14:33:21 | call to Port | url.go:33:2:33:22 | []type{args} |
| url.go:34:2:34:23 | []type{args} | url.go:34:14:34:22 | call to Query |
| url.go:34:14:34:14 | u | url.go:34:14:34:22 | call to Query |
| url.go:34:14:34:22 | call to Query | url.go:34:2:34:23 | []type{args} |
| url.go:35:14:35:14 | u | url.go:35:14:35:27 | call to RequestURI |
| url.go:35:14:35:27 | call to RequestURI | url.go:35:2:35:28 | []type{args} |
| url.go:36:6:36:6 | u | url.go:36:6:36:26 | call to ResolveReference |
| url.go:36:25:36:25 | u | url.go:36:6:36:26 | call to ResolveReference |
| url.go:41:17:41:20 | "me" | url.go:41:8:41:21 | call to User |
Expand All @@ -78,35 +58,27 @@
| url.go:43:11:43:12 | ui | url.go:43:2:43:23 | ... := ...[0] |
| url.go:43:11:43:23 | call to Password | url.go:43:2:43:23 | ... := ...[0] |
| url.go:43:11:43:23 | call to Password | url.go:43:2:43:23 | ... := ...[1] |
| url.go:44:14:44:15 | pw | url.go:44:2:44:16 | []type{args} |
| url.go:45:14:45:15 | ui | url.go:45:14:45:26 | call to Username |
| url.go:45:14:45:26 | call to Username | url.go:45:2:45:27 | []type{args} |
| url.go:50:10:50:26 | call to ParseQuery | url.go:50:2:50:26 | ... := ...[0] |
| url.go:50:10:50:26 | call to ParseQuery | url.go:50:2:50:26 | ... := ...[1] |
| url.go:50:25:50:25 | q | url.go:50:2:50:26 | ... := ...[0] |
| url.go:51:14:51:14 | v | url.go:51:14:51:23 | call to Encode |
| url.go:51:14:51:23 | call to Encode | url.go:51:2:51:24 | []type{args} |
| url.go:52:14:52:14 | v | url.go:52:14:52:26 | call to Get |
| url.go:52:14:52:26 | call to Get | url.go:52:2:52:27 | []type{args} |
| url.go:57:16:57:39 | call to JoinPath | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:16:57:39 | call to JoinPath | url.go:57:2:57:39 | ... := ...[1] |
| url.go:57:29:57:29 | q | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:32:57:38 | "clean" | url.go:57:2:57:39 | ... := ...[0] |
| url.go:57:32:57:38 | "clean" | url.go:57:16:57:39 | []type{args} |
| url.go:58:16:58:45 | call to JoinPath | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:16:58:45 | call to JoinPath | url.go:58:2:58:45 | ... := ...[1] |
| url.go:58:29:58:35 | "clean" | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:38:58:44 | joined1 | url.go:58:2:58:45 | ... := ...[0] |
| url.go:58:38:58:44 | joined1 | url.go:58:16:58:45 | []type{args} |
| url.go:59:14:59:31 | call to Parse | url.go:59:2:59:31 | ... := ...[0] |
| url.go:59:14:59:31 | call to Parse | url.go:59:2:59:31 | ... := ...[1] |
| url.go:59:24:59:30 | joined2 | url.go:59:2:59:31 | ... := ...[0] |
| url.go:60:15:60:19 | asUrl | url.go:60:15:60:37 | call to JoinPath |
| url.go:60:30:60:36 | "clean" | url.go:60:15:60:37 | []type{args} |
| url.go:60:30:60:36 | "clean" | url.go:60:15:60:37 | call to JoinPath |
| url.go:65:17:65:48 | call to Parse | url.go:65:2:65:48 | ... := ...[0] |
| url.go:65:17:65:48 | call to Parse | url.go:65:2:65:48 | ... := ...[1] |
| url.go:65:27:65:47 | "http://harmless.org" | url.go:65:2:65:48 | ... := ...[0] |
| url.go:66:9:66:16 | cleanUrl | url.go:66:9:66:28 | call to JoinPath |
| url.go:66:27:66:27 | q | url.go:66:9:66:28 | []type{args} |
| url.go:66:27:66:27 | q | url.go:66:9:66:28 | call to JoinPath |
12 changes: 1 addition & 11 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
View file
Open in desktop
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
2 changes: 0 additions & 2 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/Flows.expected
View file
Open in desktop

This file was deleted.

21 changes: 0 additions & 21 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/Flows.ext.yml
View file
Open in desktop

This file was deleted.

22 changes: 0 additions & 22 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/Flows.ql
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/go.mod
View file
Open in desktop

This file was deleted.

57 changes: 0 additions & 57 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/main.go
View file
Open in desktop

This file was deleted.

31 changes: 0 additions & 31 deletions ...sts/semmle/go/dataflow/VarArgsWithExternalFlow/vendor/github.com/nonexistent/test/stub.go
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/vendor/modules.txt
View file
Open in desktop

This file was deleted.

Loading
Loading