Revert "Merge pull request #18275 from owen-mc/go/mad/variadic-params-sources"

This reverts commit 7ab06fca2f, reversing changes made to 0c5e260ae6.
2025-12-17 09:13:20 +01:00 · 2025-01-07 14:55:06 -05:00
parent f12ff2d77f
commit 3dcf49cea0
7 changed files with 21 additions and 97 deletions
--- a/go/ql/lib/change-notes/2024-12-12-variadic-parameter-sources.md
+++ b/go/ql/lib/change-notes/2024-12-12-variadic-parameter-sources.md
@@ -1,4 +0,0 @@
---
-category: minorAnalysis
---
-* Source models defined using models-as-data now work for variadic parameters.
--- a/go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
+++ b/go/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.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
-}
--- a/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.qll
+++ b/go/ql/lib/semmle/go/dataflow/internal/TaintTrackingUtil.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. */
--- a/go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
+++ b/go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected
@@ -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 |
--- a/go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/main.go
+++ b/go/ql/test/library-tests/semmle/go/dataflow/VarArgsWithExternalFlow/main.go
@@ -44,13 +44,7 @@ func main() {

 	var variadicSource string
 	test.VariadicSource(&variadicSource)
-	sink(variadicSource)  // $ hasTaintFlow="variadicSource"
-	sink(&variadicSource) // $ hasTaintFlow="&..."
-
-	var variadicSourcePtr *string
-	test.VariadicSource(variadicSourcePtr)
-	sink(variadicSourcePtr)  // $ hasTaintFlow="variadicSourcePtr"
-	sink(*variadicSourcePtr) // $ hasTaintFlow="star expression"
+	sink(variadicSource) // $ MISSING: hasTaintFlow="variadicSource"

 	test.VariadicSink(source()) // $ hasTaintFlow="[]type{args}"

--- a/go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/TaintStep.expected
+++ b/go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/TaintStep.expected
@@ -10,13 +10,9 @@ invalidModelRow
 | io.go:14:31:14:43 | "some string" | io.go:14:13:14:44 | call to NewReader |
 | io.go:16:3:16:3 | definition of w | io.go:16:23:16:27 | &... |
 | io.go:16:3:16:3 | definition of w | io.go:16:30:16:34 | &... |
-| io.go:16:8:16:35 | []type{args} | io.go:16:23:16:27 | &... |
-| io.go:16:8:16:35 | []type{args} | io.go:16:30:16:34 | &... |
 | io.go:16:23:16:27 | &... | io.go:15:7:15:10 | definition of buf1 |
-| io.go:16:23:16:27 | &... | io.go:16:8:16:35 | []type{args} |
 | io.go:16:24:16:27 | buf1 | io.go:16:23:16:27 | &... |
 | io.go:16:30:16:34 | &... | io.go:15:13:15:16 | definition of buf2 |
-| io.go:16:30:16:34 | &... | io.go:16:8:16:35 | []type{args} |
 | io.go:16:31:16:34 | buf2 | io.go:16:30:16:34 | &... |
 | io.go:18:14:18:19 | reader | io.go:16:3:16:3 | definition of w |
 | io.go:22:31:22:43 | "some string" | io.go:22:13:22:44 | call to NewReader |
@@ -31,10 +27,8 @@ invalidModelRow
 | io.go:39:11:39:19 | call to Pipe | io.go:39:3:39:19 | ... := ...[0] |
 | io.go:39:11:39:19 | call to Pipe | io.go:39:3:39:19 | ... := ...[1] |
 | io.go:40:17:40:31 | "some string\\n" | io.go:39:6:39:6 | definition of w |
-| io.go:40:17:40:31 | "some string\\n" | io.go:40:3:40:32 | []type{args} |
 | io.go:43:16:43:16 | r | io.go:42:3:42:5 | definition of buf |
 | io.go:44:13:44:15 | buf | io.go:44:13:44:24 | call to String |
-| io.go:44:13:44:24 | call to String | io.go:44:3:44:25 | []type{args} |
 | io.go:48:31:48:43 | "some string" | io.go:48:13:48:44 | call to NewReader |
 | io.go:50:18:50:23 | reader | io.go:49:3:49:5 | definition of buf |
 | io.go:54:31:54:43 | "some string" | io.go:54:13:54:44 | call to NewReader |
@@ -52,14 +46,8 @@ invalidModelRow
 | io.go:82:27:82:36 | "reader1 " | io.go:82:9:82:37 | call to NewReader |
 | io.go:83:27:83:36 | "reader2 " | io.go:83:9:83:37 | call to NewReader |
 | io.go:84:27:84:35 | "reader3" | io.go:84:9:84:36 | call to NewReader |
-| io.go:85:8:85:33 | []type{args} | io.go:82:3:82:4 | definition of r1 |
-| io.go:85:8:85:33 | []type{args} | io.go:83:3:83:4 | definition of r2 |
-| io.go:85:8:85:33 | []type{args} | io.go:84:3:84:4 | definition of r3 |
-| io.go:85:23:85:24 | r1 | io.go:85:8:85:33 | []type{args} |
 | io.go:85:23:85:24 | r1 | io.go:85:8:85:33 | call to MultiReader |
-| io.go:85:27:85:28 | r2 | io.go:85:8:85:33 | []type{args} |
 | io.go:85:27:85:28 | r2 | io.go:85:8:85:33 | call to MultiReader |
-| io.go:85:31:85:32 | r3 | io.go:85:8:85:33 | []type{args} |
 | io.go:85:31:85:32 | r3 | io.go:85:8:85:33 | call to MultiReader |
 | io.go:86:22:86:22 | r | io.go:86:11:86:19 | selection of Stdout |
 | io.go:89:26:89:38 | "some string" | io.go:89:8:89:39 | call to NewReader |
--- a/go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected
+++ b/go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected
@@ -66,13 +66,15 @@ edges
 | passwords.go:34:28:34:35 | password | passwords.go:34:14:34:35 | ...+... | provenance | Config |
 | passwords.go:36:2:36:5 | definition of obj1 | passwords.go:39:14:39:17 | obj1 | provenance |  |
 | passwords.go:36:2:36:5 | definition of obj1 | passwords.go:39:14:39:17 | obj1 | provenance |  |
-| passwords.go:36:10:38:2 | struct literal | passwords.go:36:2:36:5 | definition of obj1 | provenance |  |
+| passwords.go:36:10:38:2 | struct literal | passwords.go:39:14:39:17 | obj1 | provenance |  |
+| passwords.go:36:10:38:2 | struct literal | passwords.go:39:14:39:17 | obj1 | provenance |  |
 | passwords.go:37:13:37:13 | x | passwords.go:36:10:38:2 | struct literal | provenance | Config |
 | passwords.go:39:2:39:18 | []type{args} [array] | passwords.go:36:2:36:5 | definition of obj1 | provenance |  |
 | passwords.go:39:14:39:17 | obj1 | passwords.go:39:2:39:18 | []type{args} [array] | provenance |  |
 | passwords.go:41:2:41:5 | definition of obj2 | passwords.go:44:14:44:17 | obj2 | provenance |  |
 | passwords.go:41:2:41:5 | definition of obj2 | passwords.go:44:14:44:17 | obj2 | provenance |  |
-| passwords.go:41:10:43:2 | struct literal | passwords.go:41:2:41:5 | definition of obj2 | provenance |  |
+| passwords.go:41:10:43:2 | struct literal | passwords.go:44:14:44:17 | obj2 | provenance |  |
+| passwords.go:41:10:43:2 | struct literal | passwords.go:44:14:44:17 | obj2 | provenance |  |
 | passwords.go:42:6:42:13 | password | passwords.go:41:10:43:2 | struct literal | provenance | Config |
 | passwords.go:44:2:44:18 | []type{args} [array] | passwords.go:41:2:41:5 | definition of obj2 | provenance |  |
 | passwords.go:44:14:44:17 | obj2 | passwords.go:44:2:44:18 | []type{args} [array] | provenance |  |
@@ -83,7 +85,8 @@ edges
 | passwords.go:48:11:48:18 | password | passwords.go:46:6:46:9 | definition of obj3 | provenance | Config |
 | passwords.go:85:2:85:14 | definition of utilityObject | passwords.go:88:14:88:26 | utilityObject | provenance |  |
 | passwords.go:85:2:85:14 | definition of utilityObject | passwords.go:88:14:88:26 | utilityObject | provenance |  |
-| passwords.go:85:19:87:2 | struct literal | passwords.go:85:2:85:14 | definition of utilityObject | provenance |  |
+| passwords.go:85:19:87:2 | struct literal | passwords.go:88:14:88:26 | utilityObject | provenance |  |
+| passwords.go:85:19:87:2 | struct literal | passwords.go:88:14:88:26 | utilityObject | provenance |  |
 | passwords.go:86:16:86:36 | call to make | passwords.go:85:19:87:2 | struct literal | provenance | Config |
 | passwords.go:88:2:88:27 | []type{args} [array] | passwords.go:85:2:85:14 | definition of utilityObject | provenance |  |
 | passwords.go:88:14:88:26 | utilityObject | passwords.go:88:2:88:27 | []type{args} [array] | provenance |  |
@@ -99,9 +102,12 @@ edges
 | passwords.go:118:2:118:7 | definition of config [x] | passwords.go:126:14:126:19 | config [x] | provenance |  |
 | passwords.go:118:2:118:7 | definition of config [y] | passwords.go:125:14:125:19 | config [y] | provenance |  |
 | passwords.go:118:2:118:7 | definition of config [y] | passwords.go:127:14:127:19 | config [y] | provenance |  |
-| passwords.go:118:12:123:2 | struct literal | passwords.go:118:2:118:7 | definition of config | provenance |  |
-| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:118:2:118:7 | definition of config [x] | provenance |  |
-| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:118:2:118:7 | definition of config [y] | provenance |  |
+| passwords.go:118:12:123:2 | struct literal | passwords.go:125:14:125:19 | config | provenance |  |
+| passwords.go:118:12:123:2 | struct literal | passwords.go:125:14:125:19 | config | provenance |  |
+| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:125:14:125:19 | config [x] | provenance |  |
+| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:126:14:126:19 | config [x] | provenance |  |
+| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:125:14:125:19 | config [y] | provenance |  |
+| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:127:14:127:19 | config [y] | provenance |  |
 | passwords.go:119:13:119:13 | x | passwords.go:118:12:123:2 | struct literal | provenance | Config |
 | passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal | provenance | Config |
 | passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal [x] | provenance |  |