Merge pull request #14778 from owen-mc/go/improve-value-flow-through-arrays

Go: improve value flow through arrays

Author: owen-mc

go/ql/lib/change-notes/2023-11-15-bug-fix-value-flow-in-array.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* A bug has been fixed that meant that value flow through an array was not tracked correctly in some circumstances. Taint flow was tracked correctly.

go/ql/lib/semmle/go/Expr.qll

@@ -724,16 +724,19 @@ class GenericTypeInstantiationExpr extends Expr {
  * ```go
  * a[1:3]
  * a[1:3:5]
+ * a[1:]
+ * a[:3]
+ * a[:]
  * ```
  */
 class SliceExpr extends @sliceexpr, Expr {
   /** Gets the base of this slice expression. */
   Expr getBase() { result = this.getChildExpr(0) }
 
-  /** Gets the lower bound of this slice expression. */
+  /** Gets the lower bound of this slice expression, if any. */
   Expr getLow() { result = this.getChildExpr(1) }
 
-  /** Gets the upper bound of this slice expression. */
+  /** Gets the upper bound of this slice expression, if any. */
   Expr getHigh() { result = this.getChildExpr(2) }
 
   /** Gets the maximum of this slice expression, if any. */

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

@@ -21,7 +21,7 @@ predicate containerStoreStep(Node node1, Node node2, Content c) {
       node2.getType() instanceof SliceType
     ) and
     (
-      exists(Write w | w.writesElement(node2, _, node1))
+      exists(Write w | w.writesElement(node2.(PostUpdateNode).getPreUpdateNode(), _, node1))
       or
       node1 = node2.(ImplicitVarargsSlice).getCallNode().getAnImplicitVarargsArgument()
     )

go/ql/test/library-tests/semmle/go/dataflow/ArrayConversion/main.go

@@ -21,5 +21,5 @@ func main() {
 	// Compare with the standard dataflow support for arrays
 	var b [4]string
 	b[0] = source()
-	sink(b[0]) // $ hasTaintFlow="index expression"
+	sink(b[0]) // $ hasValueFlow="index expression"
 }

go/ql/test/library-tests/semmle/go/frameworks/Beego/ReflectedXss.expected

@@ -47,6 +47,7 @@ edges
 | test.go:246:15:246:36 | call to GetString | test.go:249:21:249:29 | untrusted |
 | test.go:259:23:259:44 | call to GetCookie | test.go:259:16:259:45 | type conversion |
 | test.go:270:62:270:83 | call to GetCookie | test.go:270:55:270:84 | type conversion |
+| test.go:275:2:275:40 | ... := ...[0] | test.go:278:21:278:28 | index expression |
 | test.go:275:2:275:40 | ... := ...[0] | test.go:283:44:283:60 | selection of Filename |
 | test.go:275:2:275:40 | ... := ...[0] | test.go:284:38:284:49 | genericFiles |
 | test.go:275:2:275:40 | ... := ...[0] | test.go:285:37:285:48 | genericFiles |
@@ -61,6 +62,8 @@ edges
 | test.go:275:2:275:40 | ... := ...[0] | test.go:301:39:301:50 | genericFiles |
 | test.go:275:2:275:40 | ... := ...[0] | test.go:302:40:302:51 | genericFiles |
 | test.go:275:2:275:40 | ... := ...[0] | test.go:303:39:303:50 | genericFiles |
+| test.go:276:2:276:13 | definition of genericFiles [array] | test.go:297:51:297:62 | genericFiles [array] |
+| test.go:278:21:278:28 | index expression | test.go:276:2:276:13 | definition of genericFiles [array] |
 | test.go:283:44:283:60 | selection of Filename | test.go:283:21:283:61 | call to GetDisplayString |
 | test.go:284:21:284:53 | call to SliceChunk | test.go:284:21:284:92 | selection of Filename |
 | test.go:284:38:284:49 | genericFiles | test.go:284:21:284:53 | call to SliceChunk |
@@ -77,6 +80,7 @@ edges
 | test.go:296:21:296:61 | call to SliceMerge | test.go:296:21:296:97 | selection of Filename |
 | test.go:296:49:296:60 | genericFiles | test.go:296:21:296:61 | call to SliceMerge |
 | test.go:297:21:297:66 | call to SlicePad | test.go:297:21:297:102 | selection of Filename |
+| test.go:297:51:297:62 | genericFiles [array] | test.go:297:51:297:65 | index expression |
 | test.go:297:51:297:65 | index expression | test.go:297:21:297:66 | call to SlicePad |
 | test.go:298:21:298:66 | call to SlicePad | test.go:298:21:298:102 | selection of Filename |
 | test.go:298:36:298:47 | genericFiles | test.go:298:21:298:66 | call to SlicePad |
@@ -177,6 +181,8 @@ nodes
 | test.go:270:55:270:84 | type conversion | semmle.label | type conversion |
 | test.go:270:62:270:83 | call to GetCookie | semmle.label | call to GetCookie |
 | test.go:275:2:275:40 | ... := ...[0] | semmle.label | ... := ...[0] |
+| test.go:276:2:276:13 | definition of genericFiles [array] | semmle.label | definition of genericFiles [array] |
+| test.go:278:21:278:28 | index expression | semmle.label | index expression |
 | test.go:283:21:283:61 | call to GetDisplayString | semmle.label | call to GetDisplayString |
 | test.go:283:44:283:60 | selection of Filename | semmle.label | selection of Filename |
 | test.go:284:21:284:53 | call to SliceChunk | semmle.label | call to SliceChunk |
@@ -202,6 +208,7 @@ nodes
 | test.go:296:49:296:60 | genericFiles | semmle.label | genericFiles |
 | test.go:297:21:297:66 | call to SlicePad | semmle.label | call to SlicePad |
 | test.go:297:21:297:102 | selection of Filename | semmle.label | selection of Filename |
+| test.go:297:51:297:62 | genericFiles [array] | semmle.label | genericFiles [array] |
 | test.go:297:51:297:65 | index expression | semmle.label | index expression |
 | test.go:298:21:298:66 | call to SlicePad | semmle.label | call to SlicePad |
 | test.go:298:21:298:102 | selection of Filename | semmle.label | selection of Filename |