Fix bug with read/store steps and named types

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

@@ -14,38 +14,40 @@ private import semmle.go.dataflow.ExternalFlow
  * varargs.
  */
 predicate containerStoreStep(Node node1, Node node2, Content c) {
-  c instanceof ArrayContent and
-  (
+  exists(Type t | t = node2.getType().getUnderlyingType() |
+    c instanceof ArrayContent and
     (
-      node2.getType() instanceof ArrayType or
-      node2.getType() instanceof SliceType
-    ) and
-    (
-      exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
-      or
-      node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
-      or
-      // To model data flow from array elements of the base of a `SliceNode` to
-      // the `SliceNode` itself, we consider there to be a read step with array
-      // content from the base to the corresponding `SliceElementNode` and then
-      // a store step with array content from the `SliceelementNode` to the
-      // `SliceNode` itself.
-      node2 = node1.(SliceElementNode).getSliceNode()
+      (
+        t instanceof ArrayType or
+        t instanceof SliceType
+      ) and
+      (
+        exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
+        or
+        node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
+        or
+        // To model data flow from array elements of the base of a `SliceNode` to
+        // the `SliceNode` itself, we consider there to be a read step with array
+        // content from the base to the corresponding `SliceElementNode` and then
+        // a store step with array content from the `SliceelementNode` to the
+        // `SliceNode` itself.
+        node2 = node1.(SliceElementNode).getSliceNode()
+      )
     )
+    or
+    c instanceof CollectionContent and
+    exists(SendStmt send |
+      send.getChannel() = node2.(ExprNode).asExpr() and send.getValue() = node1.(ExprNode).asExpr()
+    )
+    or
+    c instanceof MapKeyContent and
+    t instanceof MapType and
+    exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), node1, _))
+    or
+    c instanceof MapValueContent and
+    t instanceof MapType and
+    exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
   )
-  or
-  c instanceof CollectionContent and
-  exists(SendStmt send |
-    send.getChannel() = node2.(ExprNode).asExpr() and send.getValue() = node1.(ExprNode).asExpr()
-  )
-  or
-  c instanceof MapKeyContent and
-  node2.getType() instanceof MapType and
-  exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), node1, _))
-  or
-  c instanceof MapValueContent and
-  node2.getType() instanceof MapType and
-  exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
 }
 
 /**
@@ -55,35 +57,37 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
  * as well as array iteration through enhanced `for` statements.
  */
 predicate containerReadStep(Node node1, Node node2, Content c) {
-  c instanceof ArrayContent and
-  (
-    node1.getType() instanceof ArrayType or
-    node1.getType() instanceof SliceType
-  ) and
-  (
-    node2.(Read).readsElement(node1, _)
+  exists(Type t | t = node1.getType().getUnderlyingType() |
+    c instanceof ArrayContent and
+    (
+      t instanceof ArrayType or
+      t instanceof SliceType
+    ) and
+    (
+      node2.(Read).readsElement(node1, _)
+      or
+      node2.(RangeElementNode).getBase() = node1
+      or
+      // To model data flow from array elements of the base of a `SliceNode` to
+      // the `SliceNode` itself, we consider there to be a read step with array
+      // content from the base to the corresponding `SliceElementNode` and then
+      // a store step with array content from the `SliceelementNode` to the
+      // `SliceNode` itself.
+      node2.(SliceElementNode).getSliceNode().getBase() = node1
+    )
     or
-    node2.(RangeElementNode).getBase() = node1
+    c instanceof CollectionContent and
+    exists(UnaryOperationNode recv | recv = node2 |
+      node1 = recv.getOperand() and
+      recv.getOperator() = "<-"
+    )
     or
-    // To model data flow from array elements of the base of a `SliceNode` to
-    // the `SliceNode` itself, we consider there to be a read step with array
-    // content from the base to the corresponding `SliceElementNode` and then
-    // a store step with array content from the `SliceelementNode` to the
-    // `SliceNode` itself.
-    node2.(SliceElementNode).getSliceNode().getBase() = node1
+    c instanceof MapKeyContent and
+    t instanceof MapType and
+    node2.(RangeIndexNode).getBase() = node1
+    or
+    c instanceof MapValueContent and
+    t instanceof MapType and
+    (node2.(Read).readsElement(node1, _) or node2.(RangeElementNode).getBase() = node1)
   )
-  or
-  c instanceof CollectionContent and
-  exists(UnaryOperationNode recv | recv = node2 |
-    node1 = recv.getOperand() and
-    recv.getOperator() = "<-"
-  )
-  or
-  c instanceof MapKeyContent and
-  node1.getType() instanceof MapType and
-  node2.(RangeIndexNode).getBase() = node1
-  or
-  c instanceof MapValueContent and
-  node1.getType() instanceof MapType and
-  (node2.(Read).readsElement(node1, _) or node2.(RangeElementNode).getBase() = node1)
 }

go/ql/test/library-tests/semmle/go/dataflow/ExternalTaintFlow/test.go

@@ -117,10 +117,10 @@ func simpleflow() {
 		b.Sink1(k) // $ hasTaintFlow="k"
 	}
 	for k, _ := range mapstringstringtype(taint12) {
-		b.Sink1(k) // $ MISSING: hasTaintFlow="k"
+		b.Sink1(k) // $ hasTaintFlow="k"
 	}
 	for k := range mapstringstringtype(taint12) {
-		b.Sink1(k) // $ MISSING: hasTaintFlow="k"
+		b.Sink1(k) // $ hasTaintFlow="k"
 	}
 
 	srcMap13 := map[string]string{src.(string): ""}
@@ -133,7 +133,7 @@ func simpleflow() {
 		b.Sink1(v) // $ hasTaintFlow="v"
 	}
 	for _, v := range mapstringstringtype(taint14) {
-		b.Sink1(v) // $ MISSING: hasTaintFlow="v"
+		b.Sink1(v) // $ hasTaintFlow="v"
 	}
 
 	srcMap15 := map[string]string{"": src.(string)}

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

@@ -87,7 +87,7 @@ func simpleflow() {
 		b.Sink1(x) // $ hasValueFlow="x"
 	}
 	for _, x := range arraytype(taint8) {
-		b.Sink1(x) // $ MISSING: hasValueFlow="x"
+		b.Sink1(x) // $ hasValueFlow="x"
 	}
 
 	srcArray := []interface{}{nil, src}
@@ -117,10 +117,10 @@ func simpleflow() {
 		b.Sink1(k) // $ hasValueFlow="k"
 	}
 	for k, _ := range mapstringstringtype(taint12) {
-		b.Sink1(k) // $ MISSING: hasValueFlow="k"
+		b.Sink1(k) // $ hasValueFlow="k"
 	}
 	for k := range mapstringstringtype(taint12) {
-		b.Sink1(k) // $ MISSING: hasValueFlow="k"
+		b.Sink1(k) // $ hasValueFlow="k"
 	}
 
 	srcMap13 := map[string]string{src.(string): ""}
@@ -133,7 +133,7 @@ func simpleflow() {
 		b.Sink1(v) // $ hasValueFlow="v"
 	}
 	for _, v := range mapstringstringtype(taint14) {
-		b.Sink1(v) // $ MISSING: hasValueFlow="v"
+		b.Sink1(v) // $ hasValueFlow="v"
 	}
 
 	srcMap15 := map[string]string{"": src.(string)}