From 9016997b511e6cef468a68ba548f7a5967f4e346 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 14 Feb 2024 14:56:24 +0000 Subject: [PATCH 1/7] Golang: fix flow from a map value via a range statement --- .../semmle/go/dataflow/internal/ContainerFlow.qll | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll b/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll index ad985e2c5b5a..a864b6a4d03b 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll +++ b/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll @@ -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) } From 7ed73bc4ed5d09cea2b4f65d5457e7bce9d0a837 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Wed, 14 Feb 2024 15:45:03 +0000 Subject: [PATCH 2/7] change note --- go/ql/lib/change-notes/2024-02-14-range-map-read.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 go/ql/lib/change-notes/2024-02-14-range-map-read.md diff --git a/go/ql/lib/change-notes/2024-02-14-range-map-read.md b/go/ql/lib/change-notes/2024-02-14-range-map-read.md new file mode 100644 index 000000000000..ea45737a72ea --- /dev/null +++ b/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. From d57160db5cc2be3d045f9f155d4e6e759878d64e Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 23 Feb 2024 16:37:26 +0000 Subject: [PATCH 3/7] Direct map stores via a post-update node --- go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll b/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll index a864b6a4d03b..e6a21a06decb 100644 --- a/go/ql/lib/semmle/go/dataflow/internal/ContainerFlow.qll +++ b/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)) } /** From 12213a0a0814131fdfe0f5a2922261726f223629 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 23 Feb 2024 18:39:16 +0000 Subject: [PATCH 4/7] Add test --- .../dataflow/MapReadsAndStores/Flows.expected | 0 .../go/dataflow/MapReadsAndStores/Flows.ql | 3 +++ .../go/dataflow/MapReadsAndStores/test.go | 17 +++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.expected create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.ql create mode 100644 go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.expected b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.ql b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.ql new file mode 100644 index 000000000000..1b27b27d6dc2 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/Flows.ql @@ -0,0 +1,3 @@ +import go +import TestUtilities.InlineFlowTest +import DefaultFlowTest diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go new file mode 100644 index 000000000000..b27443a6d895 --- /dev/null +++ b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go @@ -0,0 +1,17 @@ +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" + } +} From e62a0805db04e31b4faaa8e13066ce557e2ba1f2 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Tue, 27 Feb 2024 13:44:52 +0000 Subject: [PATCH 5/7] Add test for map literal --- .../semmle/go/dataflow/MapReadsAndStores/test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go index b27443a6d895..4a7b757d1653 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go @@ -15,3 +15,11 @@ func main() { sink(val) // $ hasValueFlow="val" } } + +func testLiteral() { + someMap := map[string]string {"someKey": source()} + + for _, val := range someMap { + sink(val) // $ hasValueFlow="val" + } +} From 74448c092a862aaf75cd87d0fa093324fb87bd78 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Tue, 27 Feb 2024 13:49:12 +0000 Subject: [PATCH 6/7] Autoformat / uglify --- .../library-tests/semmle/go/dataflow/MapReadsAndStores/test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go index 4a7b757d1653..7e42e4c038e1 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go @@ -17,7 +17,7 @@ func main() { } func testLiteral() { - someMap := map[string]string {"someKey": source()} + someMap := map[string]string{"someKey": source()} for _, val := range someMap { sink(val) // $ hasValueFlow="val" From a6480a4ca195496d655036151559ed03a8fdf665 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Tue, 27 Feb 2024 13:55:26 +0000 Subject: [PATCH 7/7] Autoformat again / tabify --- .../semmle/go/dataflow/MapReadsAndStores/test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go index 7e42e4c038e1..b6dedf748b3d 100644 --- a/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go +++ b/go/ql/test/library-tests/semmle/go/dataflow/MapReadsAndStores/test.go @@ -17,9 +17,9 @@ func main() { } func testLiteral() { - someMap := map[string]string{"someKey": source()} + someMap := map[string]string{"someKey": source()} - for _, val := range someMap { - sink(val) // $ hasValueFlow="val" - } + for _, val := range someMap { + sink(val) // $ hasValueFlow="val" + } }