Merge pull request github#18434 from github/dbartol/revert-go

Revert two Go PRs

Author: Dave Bartolomeo

go/ql/lib/change-notes/2024-12-06-improve-flow-out-of-variadic-parameter.md

@@ -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

go/ql/lib/change-notes/2024-12-12-variadic-parameter-sources.md

@@ -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

go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll

@@ -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
-}

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -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) {
@@ -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 = ""
@@ -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. */

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

@@ -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>

go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll

@@ -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] |
@@ -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 |
@@ -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 |

go/ql/test/library-tests/semmle/go/dataflow/ExternalValueFlow/completetest.ql

@@ -4,7 +4,7 @@ func source() string {
 	return "untrusted data"
 }
 
-func sink(any) {
+func sink(string) {
 }
 
 type A struct {
@@ -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
 }
@@ -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"