Merge pull request #13867 from github/mbg/go/1.21-support

Go: Basic Go 1.21 support

.github/workflows/go-tests-other-os.yml

@@ -7,15 +7,17 @@ on:
       - .github/workflows/go-tests-other-os.yml
       - .github/actions/**
       - codeql-workspace.yml
+env:
+  GO_VERSION: '~1.21.0'
 jobs:
   test-mac:
     name: Test MacOS
     runs-on: macos-latest
     steps:
-      - name: Set up Go 1.20
+      - name: Set up Go ${{ env.GO_VERSION }}
         uses: actions/setup-go@v4
         with:
-          go-version: '1.20'
+          go-version: ${{ env.GO_VERSION }}
         id: go
 
       - name: Check out code
@@ -47,10 +49,10 @@ jobs:
     name: Test Windows
     runs-on: windows-latest-xl
     steps:
-      - name: Set up Go 1.20
+      - name: Set up Go ${{ env.GO_VERSION }}
         uses: actions/setup-go@v4
         with:
-          go-version: '1.20'
+          go-version: ${{ env.GO_VERSION }}
         id: go
 
       - name: Check out code

.github/workflows/go-tests.yml

@@ -15,15 +15,17 @@ on:
       - .github/workflows/go-tests.yml
       - .github/actions/**
       - codeql-workspace.yml
+env:
+  GO_VERSION: '~1.21.0'
 jobs:
   test-linux:
     name: Test Linux (Ubuntu)
     runs-on: ubuntu-latest-xl
     steps:
-      - name: Set up Go 1.20
+      - name: Set up Go ${{ env.GO_VERSION }}
         uses: actions/setup-go@v4
         with:
-          go-version: '1.20'
+          go-version: ${{ env.GO_VERSION }}
         id: go
 
       - name: Check out code

go/extractor/cli/go-autobuilder/go-autobuilder.go

@@ -830,7 +830,7 @@ func installDependenciesAndBuild() {
 }
 
 const minGoVersion = "1.11"
-const maxGoVersion = "1.20"
+const maxGoVersion = "1.21"
 
 // Check if `version` is lower than `minGoVersion`. Note that for this comparison we ignore the
 // patch part of the version, so 1.20.1 and 1.20 are considered equal.

go/go.mod

@@ -1,6 +1,6 @@
 module github.com/github/codeql-go
 
-go 1.20
+go 1.21
 
 require (
 	golang.org/x/mod v0.12.0

go/ql/lib/semmle/go/Scopes.qll

@@ -678,6 +678,8 @@ private predicate builtinFunction(
   or
   name = "cap" and pure = true and mayPanic = false and mustPanic = false and variadic = false
   or
+  name = "clear" and pure = false and mayPanic = false and mustPanic = false and variadic = false
+  or
   name = "close" and pure = false and mayPanic = true and mustPanic = false and variadic = false
   or
   name = "complex" and pure = true and mayPanic = true and mustPanic = false and variadic = false
@@ -692,6 +694,10 @@ private predicate builtinFunction(
   or
   name = "make" and pure = true and mayPanic = true and mustPanic = false and variadic = true
   or
+  name = "max" and pure = true and mayPanic = false and mustPanic = false and variadic = true
+  or
+  name = "min" and pure = true and mayPanic = false and mustPanic = false and variadic = true
+  or
   name = "new" and pure = true and mayPanic = false and mustPanic = false and variadic = false
   or
   name = "panic" and pure = false and mayPanic = true and mustPanic = true and variadic = false
@@ -795,6 +801,9 @@ module Builtin {
   /** Gets the built-in function `cap`. */
   BuiltinFunction cap() { result.getName() = "cap" }
 
+  /** Gets the built-in function `clear`. */
+  BuiltinFunction clear() { result.getName() = "clear" }
+
   /** Gets the built-in function `close`. */
   BuiltinFunction close() { result.getName() = "close" }
 
@@ -816,6 +825,12 @@ module Builtin {
   /** Gets the built-in function `make`. */
   BuiltinFunction make() { result.getName() = "make" }
 
+  /** Gets the built-in function `max`. */
+  BuiltinFunction max_() { result.getName() = "max" }
+
+  /** Gets the built-in function `min`. */
+  BuiltinFunction min_() { result.getName() = "min" }
+
   /** Gets the built-in function `new`. */
   BuiltinFunction new() { result.getName() = "new" }
 

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

@@ -408,3 +408,19 @@ class ListOfConstantsComparisonSanitizerGuard extends TaintTracking::DefaultTain
     this = DataFlow::BarrierGuard<listOfConstantsComparisonSanitizerGuard/3>::getABarrierNode()
   }
 }
+
+/**
+ * The `clear` built-in function deletes or zeroes out all elements of a map or slice
+ * and therefore acts as a general sanitizer for taint flow to any uses dominated by it.
+ */
+private class ClearSanitizer extends DefaultTaintSanitizer {
+  ClearSanitizer() {
+    exists(SsaWithFields var, DataFlow::CallNode call, DataFlow::Node arg | this = var.getAUse() |
+      call = Builtin::clear().getACall() and
+      arg = call.getAnArgument() and
+      arg = var.getAUse() and
+      arg != this and
+      this.getBasicBlock().(ReachableBasicBlock).dominates(this.getBasicBlock())
+    )
+  }
+}

go/ql/lib/semmle/go/frameworks/Stdlib.qll

@@ -70,6 +70,32 @@ private class CopyFunction extends TaintTracking::FunctionModel {
   }
 }
 
+/**
+ * A model of the built-in `min` function, which computes the smallest value of a fixed number of
+ * arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
+ * strings, so we care about data flow through `min`.
+ */
+private class MinFunction extends DataFlow::FunctionModel {
+  MinFunction() { this = Builtin::min_() }
+
+  override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
+    inp.isParameter(_) and outp.isResult()
+  }
+}
+
+/**
+ * A model of the built-in `max` function, which computes the largest value of a fixed number of
+ * arguments of ordered types. There is at least one argument and "ordered types" includes e.g.
+ * strings, so we care about data flow through `max`.
+ */
+private class MaxFunction extends DataFlow::FunctionModel {
+  MaxFunction() { this = Builtin::max_() }
+
+  override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp) {
+    inp.isParameter(_) and outp.isResult()
+  }
+}
+
 /** Provides a class for modeling functions which convert strings into integers. */
 module IntegerParser {
   /**

go/ql/test/library-tests/semmle/go/Function/TypeParamType.expected

@@ -1,5 +1,6 @@
 | Edge | EdgeConstraint |
 | Edge | interface { } |
+| F | floaty |
 | K | comparable |
 | Node | NodeConstraint |
 | Node | interface { } |
@@ -7,6 +8,8 @@
 | SF2 | interface { } |
 | SG2 | interface { } |
 | T | interface { } |
+| T1 | interface { } |
+| T2 | interface { } |
 | TF1 | interface { } |
 | TF2 | interface { } |
 | TG1 | interface { } |

go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/Builtin.go

@@ -0,0 +1,16 @@
+package main
+
+import "net/http"
+
+func clearTestBad(sourceReq *http.Request) string {
+	b := make([]byte, 8)
+	sourceReq.Body.Read(b)
+	return string(b)
+}
+
+func clearTestGood(sourceReq *http.Request) string {
+	b := make([]byte, 8)
+	sourceReq.Body.Read(b)
+	clear(b) // should prevent taint flow
+	return string(b)
+}

go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/DefaultSanitizer.expected

@@ -0,0 +1,10 @@
+edges
+| Builtin.go:6:2:6:2 | definition of b | Builtin.go:8:9:8:17 | type conversion |
+| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b |
+nodes
+| Builtin.go:6:2:6:2 | definition of b | semmle.label | definition of b |
+| Builtin.go:7:2:7:15 | selection of Body | semmle.label | selection of Body |
+| Builtin.go:8:9:8:17 | type conversion | semmle.label | type conversion |
+subpaths
+#select
+| Builtin.go:8:9:8:17 | type conversion | Builtin.go:7:2:7:15 | selection of Body | Builtin.go:8:9:8:17 | type conversion | Found taint flow |

go/ql/test/library-tests/semmle/go/dataflow/DefaultTaintSanitizer/DefaultSanitizer.ql

@@ -0,0 +1,20 @@
+/**
+ * @description Check that DefaultTaintSanitizer instances prevent taint flow.
+ * @kind path-problem
+ */
+
+import go
+
+module Config implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node n) { n instanceof UntrustedFlowSource }
+
+  predicate isSink(DataFlow::Node n) { any(ReturnStmt s).getAnExpr() = n.asExpr() }
+}
+
+module Flow = TaintTracking::Global<Config>;
+
+import Flow::PathGraph
+
+from Flow::PathNode source, Flow::PathNode sink
+where Flow::flowPath(source, sink)
+select sink.getNode(), source, sink, "Found taint flow"

go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalFlowStep.expected

@@ -44,6 +44,8 @@
 | file://:0:0:0:0 | function append | main.go:40:8:40:13 | append |
 | file://:0:0:0:0 | function copy | main.go:42:2:42:5 | copy |
 | file://:0:0:0:0 | function make | main.go:41:8:41:11 | make |
+| file://:0:0:0:0 | function max | main.go:65:7:65:9 | max |
+| file://:0:0:0:0 | function min | main.go:64:7:64:9 | min |
 | main.go:3:6:3:10 | function test1 | main.go:34:2:34:6 | test1 |
 | main.go:3:12:3:12 | argument corresponding to x | main.go:3:12:3:12 | definition of x |
 | main.go:3:12:3:12 | definition of x | main.go:5:5:5:5 | x |
@@ -112,6 +114,25 @@
 | main.go:55:6:55:7 | definition of ch | main.go:56:2:56:3 | ch |
 | main.go:55:6:55:7 | definition of ch | main.go:57:4:57:5 | ch |
 | main.go:55:6:55:7 | zero value for ch | main.go:55:6:55:7 | definition of ch |
+| main.go:61:2:61:2 | definition of x | main.go:64:11:64:11 | x |
+| main.go:61:2:61:2 | definition of x | main.go:65:11:65:11 | x |
+| main.go:61:7:61:7 | 1 | main.go:61:2:61:2 | definition of x |
+| main.go:62:2:62:2 | definition of y | main.go:64:14:64:14 | y |
+| main.go:62:2:62:2 | definition of y | main.go:65:14:65:14 | y |
+| main.go:62:7:62:7 | 2 | main.go:62:2:62:2 | definition of y |
+| main.go:63:2:63:2 | definition of z | main.go:64:17:64:17 | z |
+| main.go:63:2:63:2 | definition of z | main.go:65:17:65:17 | z |
+| main.go:63:7:63:7 | 3 | main.go:63:2:63:2 | definition of z |
+| main.go:64:2:64:2 | definition of a | main.go:66:9:66:9 | a |
+| main.go:64:7:64:18 | call to min | main.go:64:2:64:2 | definition of a |
+| main.go:64:11:64:11 | x | main.go:64:7:64:18 | call to min |
+| main.go:64:14:64:14 | y | main.go:64:7:64:18 | call to min |
+| main.go:64:17:64:17 | z | main.go:64:7:64:18 | call to min |
+| main.go:65:2:65:2 | definition of b | main.go:66:12:66:12 | b |
+| main.go:65:7:65:18 | call to max | main.go:65:2:65:2 | definition of b |
+| main.go:65:11:65:11 | x | main.go:65:7:65:18 | call to max |
+| main.go:65:14:65:14 | y | main.go:65:7:65:18 | call to max |
+| main.go:65:17:65:17 | z | main.go:65:7:65:18 | call to max |
 | strings.go:8:12:8:12 | argument corresponding to s | strings.go:8:12:8:12 | definition of s |
 | strings.go:8:12:8:12 | definition of s | strings.go:9:24:9:24 | s |
 | strings.go:8:12:8:12 | definition of s | strings.go:10:27:10:27 | s |

go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected

@@ -231,6 +231,8 @@
 | file://:0:0:0:0 | [summary param] -1 in ReadAt | file://:0:0:0:0 | [summary] to write: Argument[0] in ReadAt |
 | file://:0:0:0:0 | [summary param] -1 in ReadDir | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadDir |
 | file://:0:0:0:0 | [summary param] -1 in ReadDir | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadDir |
+| file://:0:0:0:0 | [summary param] -1 in ReadDir | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadDir |
+| file://:0:0:0:0 | [summary param] -1 in ReadFile | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadFile |
 | file://:0:0:0:0 | [summary param] -1 in ReadFile | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadFile |
 | file://:0:0:0:0 | [summary param] -1 in ReadFile | file://:0:0:0:0 | [summary] to write: ReturnValue in ReadFile |
 | file://:0:0:0:0 | [summary param] -1 in Recv | file://:0:0:0:0 | [summary] to write: ReturnValue in Recv |
@@ -263,6 +265,10 @@
 | file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
 | file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
 | file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
+| file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
+| file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
+| file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
+| file://:0:0:0:0 | [summary param] -1 in String | file://:0:0:0:0 | [summary] to write: ReturnValue in String |
 | file://:0:0:0:0 | [summary param] -1 in Sub | file://:0:0:0:0 | [summary] to write: ReturnValue in Sub |
 | file://:0:0:0:0 | [summary param] -1 in Sub | file://:0:0:0:0 | [summary] to write: ReturnValue in Sub |
 | file://:0:0:0:0 | [summary param] -1 in Swap | file://:0:0:0:0 | [summary] to write: ReturnValue in Swap |

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

@@ -56,3 +56,12 @@ func testch() {
 	ch <- true
 	<-ch
 }
+
+func testMinMax() (int, int) {
+	x := 1
+	y := 2
+	z := 3
+	a := min(x, y, z)
+	b := max(x, y, z)
+	return a, b
+}

go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/TaintStep.ql

@@ -1,6 +1,21 @@
+// This test finds taint tracking steps which are not data flow steps
+// to illustrate which steps are added specifically by taint tracking
 import go
 
-from DataFlow::Node pred, DataFlow::Node succ
+predicate hasLocation(DataFlow::Node node, string loc) {
+  node.hasLocationInfo(loc, _, _, _, _) and loc != ""
+  or
+  exists(string pkg, string name |
+    node.(DataFlow::SummarizedParameterNode)
+        .getCallable()
+        .asSummarizedCallable()
+        .asFunction()
+        .hasQualifiedName(pkg, name) and
+    loc = pkg + "." + name
+  )
+}
+
+from string predLoc, DataFlow::Node pred, DataFlow::Node succ
 where
   TaintTracking::localTaintStep(pred, succ) and
   not DataFlow::localFlowStep(pred, succ) and
@@ -17,6 +32,10 @@ where
     pkg.matches("crypto/rand.%") and
     name = "Read"
     or
-    pkg = ["os.dirEntry", "os.unixDirent"] and name = ["Info", "Name"]
-  )
-select pred, succ
+    pkg = ["os.dirEntry", "os.unixDirent"] and name = ["Info", "Name", "String"]
+    or
+    // Not available on arm64
+    pkg = "vendor/golang.org/x/crypto/internal/poly1305.mac" and name = "Write"
+  ) and
+  hasLocation(pred, predLoc)
+select predLoc, pred, succ

go/ql/test/library-tests/semmle/go/frameworks/TaintSteps/main.go

@@ -28,3 +28,10 @@ func readTest(req *http.Request) string {
 	req.Body.Read(b)
 	return string(b)
 }
+
+func clearTest(req *http.Request) string {
+	b := make([]byte, 8)
+	req.Body.Read(b)
+	clear(b) // should prevent taint flow
+	return string(b)
+}