Merge pull request #15613 from smowton/smowton/fix/golang-map-range-read-dataflow

Golang: fix flow from a map value via a range statement

Author: smowton

go/ql/lib/change-notes/2024-02-14-range-map-read.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* Fixed dataflow out of a `map` using a `range` statement.

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

@@ -41,11 +41,11 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
   or
   c instanceof MapKeyContent and
   node2.getType() instanceof MapType and
-  exists(Write w | w.writesElement(node2, node1, _))
+  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, _, node1))
+  exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
 }
 
 /**
@@ -57,11 +57,11 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
 predicate containerReadStep(Node node1, Node node2, Content c) {
   c instanceof ArrayContent and
   (
-    node2.(Read).readsElement(node1, _) and
-    (
-      node1.getType() instanceof ArrayType or
-      node1.getType() instanceof SliceType
-    )
+    node1.getType() instanceof ArrayType or
+    node1.getType() instanceof SliceType
+  ) and
+  (
+    node2.(Read).readsElement(node1, _)
     or
     node2.(RangeElementNode).getBase() = node1
     or
@@ -85,5 +85,5 @@ predicate containerReadStep(Node node1, Node node2, Content c) {
   or
   c instanceof MapValueContent and
   node1.getType() instanceof MapType and
-  node2.(Read).readsElement(node1, _)
+  (node2.(Read).readsElement(node1, _) or node2.(RangeElementNode).getBase() = node1)
 }

go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.expected

@@ -0,0 +1,3 @@
+import go
+import TestUtilities.InlineFlowTest
+import DefaultFlowTest

go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.ql

@@ -0,0 +1,25 @@
+package main
+
+func source() string {
+	return "untrusted data"
+}
+
+func sink(any) {
+}
+
+func main() {
+	var someMap map[string]string = map[string]string{}
+	someMap["someKey"] = source()
+
+	for _, val := range someMap {
+		sink(val) // $ hasValueFlow="val"
+	}
+}
+
+func testLiteral() {
+	someMap := map[string]string{"someKey": source()}
+
+	for _, val := range someMap {
+		sink(val) // $ hasValueFlow="val"
+	}
+}