From 038f951e9f50420ced66ece7fcfb4dfb60eca0d8 Mon Sep 17 00:00:00 2001
From: Owen Mansel-Chan <owen-mc@github.com>
Date: Sat, 23 Oct 2021 21:11:37 +0100
Subject: [PATCH] Fix containerStoreStep

Update some comments as well, and change a variable name
---
 .../go/dataflow/internal/ContainerFlow.qll       | 16 +++++++++-------
 .../go/dataflow/internal/DataFlowPrivate.qll     | 16 ++++++++--------
 .../semmle/go/dataflow/VarArgs/main.go           |  4 ++--
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll b/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
index effd3b5c9ad..0122c6800f1 100644
--- a/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
+++ b/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll
@@ -7,18 +7,19 @@ private import DataFlowUtil
 
 /**
  * Holds if the step from `node1` to `node2` stores a value in a slice or array.
- * This covers array assignments and initializers as well as implicit array
- * creations for varargs.
+ * Thus, `node2` references an object with a content `c` that contains the value
+ * of `node1`. This covers array assignments and initializers as well as
+ * implicit array creations for varargs.
  */
 predicate containerStoreStep(Node node1, Node node2, Content c) {
   c instanceof ArrayContent and
   (
     // currently there is no database information about variadic functions
     (
-      node1.getType() instanceof ArrayType or
-      node1.getType() instanceof SliceType
+      node2.getType() instanceof ArrayType or
+      node2.getType() instanceof SliceType
     ) and
-    exists(Write w | w.writesElement(node1, _, node2))
+    exists(Write w | w.writesElement(node2, _, node1))
   )
   or
   c instanceof CollectionContent and
@@ -35,8 +36,9 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
 
 /**
  * Holds if the step from `node1` to `node2` reads a value from a slice or array.
- * This covers ordinary array reads as well as array iteration through enhanced
- * `for` statements.
+ * Thus, `node1` references an object with a content `c` whose value ends up in
+ * `node2`. This covers ordinary array reads as well as array iteration through
+ * enhanced `for` statements.
  */
 predicate containerReadStep(Node node1, Node node2, Content c) {
   c instanceof ArrayContent and
diff --git a/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll b/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
index 5bc1ef5e8e1..6bfd37a5fe4 100644
--- a/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
+++ b/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll
@@ -106,7 +106,7 @@ predicate jumpStep(Node n1, Node n2) {
 
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to `c`.
- * Thus, `node2` references an object with a field `f` that contains the
+ * Thus, `node2` references an object with a content `x` that contains the
  * value of `node1`.
  */
 predicate storeStep(Node node1, Content c, PostUpdateNode node2) {
@@ -131,23 +131,23 @@ predicate storeStep(Node node1, Content c, PostUpdateNode node2) {
 }
 
 /**
- * Holds if data can flow from `node1` to `node2` via a read of `f`.
- * Thus, `node1` references an object with a field `f` whose value ends up in
+ * Holds if data can flow from `node1` to `node2` via a read of `c`.
+ * Thus, `node1` references an object with a content `c` whose value ends up in
  * `node2`.
  */
-predicate readStep(Node node1, Content f, Node node2) {
+predicate readStep(Node node1, Content c, Node node2) {
   node1 = node2.(PointerDereferenceNode).getOperand() and
-  f = any(DataFlow::PointerContent pc | pc.getPointerType() = node1.getType())
+  c = any(DataFlow::PointerContent pc | pc.getPointerType() = node1.getType())
   or
   exists(FieldReadNode read |
     node2 = read and
     node1 = read.getBase() and
-    f = any(DataFlow::FieldContent fc | fc.getField() = read.getField())
+    c = any(DataFlow::FieldContent fc | fc.getField() = read.getField())
   )
   or
-  FlowSummaryImpl::Private::Steps::summaryReadStep(node1, f, node2)
+  FlowSummaryImpl::Private::Steps::summaryReadStep(node1, c, node2)
   or
-  containerReadStep(node1, node2, f)
+  containerReadStep(node1, node2, c)
 }
 
 /**
diff --git a/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go b/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
index 37e4476e655..c16b6605452 100644
--- a/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
+++ b/ql/test/library-tests/semmle/go/dataflow/VarArgs/main.go
@@ -17,10 +17,10 @@ func functionWithVarArgsOfStructsParameter(s ...A) {
 
 func main() {
 	stringSlice := []string{source()}
-	sink(stringSlice[0]) // $ taintflow MISSING: dataflow
+	sink(stringSlice[0]) // $ taintflow dataflow
 
 	arrayOfStructs := []A{{f: source()}}
-	sink(arrayOfStructs[0].f) // $ MISSING: taintflow dataflow
+	sink(arrayOfStructs[0].f) // $ taintflow dataflow
 
 	a := A{f: source()}
 	functionWithVarArgsOfStructsParameter(a)