Go: Add file sources

go/ql/lib/change-notes/2024-08-12-add-file-models.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Local source models have been added for the APIs which open files in the `io/fs`, `io/ioutil` and `os` packages in the Go standard library. You can optionally include threat models as appropriate when using the CodeQL CLI and in GitHub code scanning. For more information, see [Analyzing your code with CodeQL queries](https://docs.github.com/code-security/codeql-cli/getting-started-with-the-codeql-cli/analyzing-your-code-with-codeql-queries#including-model-packs-to-add-potential-sources-of-tainted-data>) and [Customizing your advanced setup for code scanning](https://docs.github.com/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#extending-codeql-coverage-with-threat-models).

go/ql/lib/ext/io.fs.model.yml

@@ -10,8 +10,16 @@ extensions:
       - ["io/fs", "", False, "Sub", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "DirEntry", True, "Info", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "DirEntry", True, "Name", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["io/fs", "File", True, "Read", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
       - ["io/fs", "FS", True, "Open", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "GlobFS", True, "Glob", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "ReadDirFS", True, "ReadDir", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "ReadFileFS", True, "ReadFile", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
       - ["io/fs", "SubFS", True, "Sub", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["io/fs", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]
+      - ["io/fs", "ReadFileFS", True, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]
+      - ["io/fs", "FS", True, "Open", "", "", "ReturnValue[0]", "file", "manual"]

go/ql/lib/ext/io.ioutil.model.yml

@@ -14,3 +14,8 @@ extensions:
     data:
       - ["io/ioutil", "", False, "NopCloser", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["io/ioutil", "", False, "ReadAll", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["io/ioutil", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]

go/ql/lib/ext/os.model.yml

@@ -40,3 +40,12 @@ extensions:
       - ["os", "", False, "ExpandEnv", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["os", "", False, "NewFile", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["os", "File", True, "Fd", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
+      - ["os", "File", True, "Read", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
+      - ["os", "File", True, "ReadAt", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
+  - addsTo:
+      pack: codeql/go-all
+      extensible: sourceModel
+    data:
+      - ["os", "", False, "Open", "", "", "ReturnValue[0]", "file", "manual"]
+      - ["os", "", False, "OpenFile", "", "", "ReturnValue[0]", "file", "manual"]
+      - ["os", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]

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

@@ -1,13 +1,21 @@
 models
-| 1 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
-| 2 | Source: net/http; Request; true; Body; ; ; ; remote; manual |
+| 1 | Summary: io/fs; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
+| 2 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
+| 3 | Source: net/http; Request; true; Body; ; ; ; remote; manual |
+| 4 | Summary: os; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
 edges
 | Builtin.go:6:2:6:2 | definition of b | Builtin.go:8:9:8:17 | type conversion | provenance |  |
-| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
+| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
+| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
+| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
 | Builtin.go:12:2:12:2 | definition of b | Builtin.go:17:9:17:17 | type conversion | provenance |  |
-| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
+| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
+| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
+| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
 | Builtin.go:21:2:21:2 | definition of b | Builtin.go:24:10:24:18 | type conversion | provenance |  |
-| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
+| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
+| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
+| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
 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 |

go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/file/test.expected

@@ -0,0 +1,3 @@
+testFailures
+invalidModelRow
+failures

go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/file/test.ext.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/threat-models
+      extensible: threatModelConfiguration
+    data:
+      - ["file", true, 0]

go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/file/test.go

@@ -0,0 +1,70 @@
+package test
+
+import (
+	"io/fs"
+	"io/ioutil"
+	"os"
+)
+
+func open() {
+	file, err := os.Open("file.txt") // $ source
+	if err != nil {
+		return
+	}
+	defer file.Close()
+	file.Read([]byte{1, 2, 3})
+}
+
+func openFile() {
+	file, err := os.OpenFile("file.txt", os.O_RDWR, 0) // $source
+	if err != nil {
+		return
+	}
+	defer file.Close()
+	file.Read([]byte{1, 2, 3})
+}
+
+func readFile() {
+	data, err := os.ReadFile("file.txt") // $source
+	if err != nil {
+		return
+	}
+	_ = data
+}
+
+func readFileIoUtil() {
+	data, err := ioutil.ReadFile("file.txt") // $source
+	if err != nil {
+		return
+	}
+	_ = data
+}
+
+func getFileFS() fs.ReadFileFS {
+	return nil
+}
+
+func readFileFs() {
+	data, err := fs.ReadFile(os.DirFS("."), "file.txt") // $source
+	if err != nil {
+		return
+	}
+	_ = data
+
+	dir := getFileFS()
+	data, err = dir.ReadFile("file.txt") // $source
+
+	if err != nil {
+		return
+	}
+	_ = data
+}
+
+func fsOpen() {
+	file, err := os.DirFS(".").Open("file.txt") // $source
+	if err != nil {
+		return
+	}
+	defer file.Close()
+	file.Read([]byte{1, 2, 3})
+}

go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/file/test.ql

@@ -0,0 +1,19 @@
+import go
+import ModelValidation
+import TestUtilities.InlineExpectationsTest
+
+module SourceTest implements TestSig {
+  string getARelevantTag() { result = "source" }
+
+  predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(ThreatModelFlowSource s |
+      s.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
+        location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
+      element = s.toString() and
+      value = "" and
+      tag = "source"
+    )
+  }
+}
+
+import MakeTest<SourceTest>

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

@@ -28,15 +28,19 @@ edges
 | test.go:51:2:51:30 | ... := ...[0] | test.go:52:16:52:37 | index expression | provenance | Src:MaD:10  |
 | test.go:57:2:57:46 | ... := ...[0] | test.go:58:13:58:22 | fileHeader | provenance | Src:MaD:11  |
 | test.go:58:2:58:29 | ... := ...[0] | test.go:60:2:60:5 | file | provenance |  |
-| test.go:58:13:58:22 | fileHeader | test.go:58:2:58:29 | ... := ...[0] | provenance | MaD:16 |
+| test.go:58:13:58:22 | fileHeader | test.go:58:2:58:29 | ... := ...[0] | provenance | MaD:17 |
 | test.go:59:2:59:7 | definition of buffer | test.go:61:20:61:25 | buffer | provenance |  |
 | test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:15 |
+| test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:16 |
+| test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:18 |
 | test.go:66:2:66:31 | ... := ...[0] | test.go:67:16:67:41 | index expression | provenance | Src:MaD:12  |
 | test.go:72:2:72:31 | ... := ...[0] | test.go:74:13:74:22 | fileHeader | provenance | Src:MaD:12  |
 | test.go:74:2:74:29 | ... := ...[0] | test.go:76:2:76:5 | file | provenance |  |
-| test.go:74:13:74:22 | fileHeader | test.go:74:2:74:29 | ... := ...[0] | provenance | MaD:16 |
+| test.go:74:13:74:22 | fileHeader | test.go:74:2:74:29 | ... := ...[0] | provenance | MaD:17 |
 | test.go:75:2:75:7 | definition of buffer | test.go:77:20:77:25 | buffer | provenance |  |
 | test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:15 |
+| test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:16 |
+| test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:18 |
 | test.go:82:2:82:32 | ... := ...[0] | test.go:83:16:83:24 | selection of Value | provenance | Src:MaD:13  |
 | test.go:88:13:88:25 | call to Cookies | test.go:89:16:89:31 | selection of Value | provenance | Src:MaD:14  |
 | test.go:99:11:99:15 | &... | test.go:100:16:100:21 | selection of s | provenance | Src:MaD:3  |
@@ -49,7 +53,7 @@ edges
 | test.go:136:11:136:32 | call to Param | test.go:137:29:137:41 | type conversion | provenance | Src:MaD:4  |
 | test.go:148:11:148:32 | call to Param | test.go:149:30:149:34 | param | provenance | Src:MaD:4  |
 | test.go:149:12:149:35 | call to NewReader | test.go:150:31:150:36 | reader | provenance |  |
-| test.go:149:30:149:34 | param | test.go:149:12:149:35 | call to NewReader | provenance | MaD:17 |
+| test.go:149:30:149:34 | param | test.go:149:12:149:35 | call to NewReader | provenance | MaD:19 |
 | test.go:164:11:164:32 | call to Param | test.go:165:23:165:35 | type conversion | provenance | Src:MaD:4  |
 models
 | 1 | Summary: github.com/labstack/echo; Context; true; Get; ; ; Argument[receiver]; ReturnValue; taint; manual |
@@ -66,9 +70,11 @@ models
 | 12 | Source: github.com/labstack/echo; Context; true; MultipartForm; ; ; ReturnValue[0]; remote; manual |
 | 13 | Source: github.com/labstack/echo; Context; true; Cookie; ; ; ReturnValue[0]; remote; manual |
 | 14 | Source: github.com/labstack/echo; Context; true; Cookies; ; ; ReturnValue[0]; remote; manual |
-| 15 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
-| 16 | Summary: mime/multipart; FileHeader; true; Open; ; ; Argument[receiver]; ReturnValue[0]; taint; manual |
-| 17 | Summary: strings; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual |
+| 15 | Summary: io/fs; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
+| 16 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
+| 17 | Summary: mime/multipart; FileHeader; true; Open; ; ; Argument[receiver]; ReturnValue[0]; taint; manual |
+| 18 | Summary: os; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
+| 19 | Summary: strings; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual |
 nodes
 | test.go:15:11:15:32 | call to Param | semmle.label | call to Param |
 | test.go:16:16:16:20 | param | semmle.label | param |

go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/UnhandledCloseWritableHandle.expected

@@ -1,19 +1,30 @@
+#select
+| tests.go:9:8:9:8 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
+| tests.go:9:8:9:8 | f | tests.go:45:5:45:76 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:45:15:45:76 | call to OpenFile | call to OpenFile |
+| tests.go:14:3:14:3 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
+| tests.go:14:3:14:3 | f | tests.go:45:5:45:76 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:45:15:45:76 | call to OpenFile | call to OpenFile |
+| tests.go:56:3:56:3 | f | tests.go:54:5:54:78 | ... := ...[0] | tests.go:56:3:56:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:54:15:54:78 | call to OpenFile | call to OpenFile |
+| tests.go:68:3:68:3 | f | tests.go:66:5:66:76 | ... := ...[0] | tests.go:68:3:68:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:66:15:66:76 | call to OpenFile | call to OpenFile |
+| tests.go:110:9:110:9 | f | tests.go:108:5:108:78 | ... := ...[0] | tests.go:110:9:110:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:108:15:108:78 | call to OpenFile | call to OpenFile |
+| tests.go:129:3:129:3 | f | tests.go:125:5:125:78 | ... := ...[0] | tests.go:129:3:129:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:125:15:125:78 | call to OpenFile | call to OpenFile |
 edges
 | tests.go:8:24:8:24 | definition of f | tests.go:9:8:9:8 | f | provenance |  |
 | tests.go:12:32:12:32 | definition of f | tests.go:13:13:13:13 | capture variable f | provenance |  |
 | tests.go:13:13:13:13 | capture variable f | tests.go:14:3:14:3 | f | provenance |  |
-| tests.go:31:5:31:78 | ... := ...[0] | tests.go:32:21:32:21 | f | provenance |  |
-| tests.go:31:5:31:78 | ... := ...[0] | tests.go:33:29:33:29 | f | provenance |  |
+| tests.go:31:5:31:78 | ... := ...[0] | tests.go:32:21:32:21 | f | provenance | Src:MaD:1  |
+| tests.go:31:5:31:78 | ... := ...[0] | tests.go:33:29:33:29 | f | provenance | Src:MaD:1  |
 | tests.go:32:21:32:21 | f | tests.go:8:24:8:24 | definition of f | provenance |  |
 | tests.go:33:29:33:29 | f | tests.go:12:32:12:32 | definition of f | provenance |  |
-| tests.go:45:5:45:76 | ... := ...[0] | tests.go:46:21:46:21 | f | provenance |  |
-| tests.go:45:5:45:76 | ... := ...[0] | tests.go:47:29:47:29 | f | provenance |  |
+| tests.go:45:5:45:76 | ... := ...[0] | tests.go:46:21:46:21 | f | provenance | Src:MaD:1  |
+| tests.go:45:5:45:76 | ... := ...[0] | tests.go:47:29:47:29 | f | provenance | Src:MaD:1  |
 | tests.go:46:21:46:21 | f | tests.go:8:24:8:24 | definition of f | provenance |  |
 | tests.go:47:29:47:29 | f | tests.go:12:32:12:32 | definition of f | provenance |  |
-| tests.go:54:5:54:78 | ... := ...[0] | tests.go:56:3:56:3 | f | provenance |  |
-| tests.go:66:5:66:76 | ... := ...[0] | tests.go:68:3:68:3 | f | provenance |  |
-| tests.go:108:5:108:78 | ... := ...[0] | tests.go:110:9:110:9 | f | provenance |  |
-| tests.go:125:5:125:78 | ... := ...[0] | tests.go:129:3:129:3 | f | provenance |  |
+| tests.go:54:5:54:78 | ... := ...[0] | tests.go:56:3:56:3 | f | provenance | Src:MaD:1  |
+| tests.go:66:5:66:76 | ... := ...[0] | tests.go:68:3:68:3 | f | provenance | Src:MaD:1  |
+| tests.go:108:5:108:78 | ... := ...[0] | tests.go:110:9:110:9 | f | provenance | Src:MaD:1  |
+| tests.go:125:5:125:78 | ... := ...[0] | tests.go:129:3:129:3 | f | provenance | Src:MaD:1  |
+models
+| 1 | Source: os; ; false; OpenFile; ; ; ReturnValue[0]; file; manual |
 nodes
 | tests.go:8:24:8:24 | definition of f | semmle.label | definition of f |
 | tests.go:9:8:9:8 | f | semmle.label | f |
@@ -35,12 +46,3 @@ nodes
 | tests.go:125:5:125:78 | ... := ...[0] | semmle.label | ... := ...[0] |
 | tests.go:129:3:129:3 | f | semmle.label | f |
 subpaths
-#select
-| tests.go:9:8:9:8 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
-| tests.go:9:8:9:8 | f | tests.go:45:5:45:76 | ... := ...[0] | tests.go:9:8:9:8 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:45:15:45:76 | call to OpenFile | call to OpenFile |
-| tests.go:14:3:14:3 | f | tests.go:31:5:31:78 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:31:15:31:78 | call to OpenFile | call to OpenFile |
-| tests.go:14:3:14:3 | f | tests.go:45:5:45:76 | ... := ...[0] | tests.go:14:3:14:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:45:15:45:76 | call to OpenFile | call to OpenFile |
-| tests.go:56:3:56:3 | f | tests.go:54:5:54:78 | ... := ...[0] | tests.go:56:3:56:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:54:15:54:78 | call to OpenFile | call to OpenFile |
-| tests.go:68:3:68:3 | f | tests.go:66:5:66:76 | ... := ...[0] | tests.go:68:3:68:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:66:15:66:76 | call to OpenFile | call to OpenFile |
-| tests.go:110:9:110:9 | f | tests.go:108:5:108:78 | ... := ...[0] | tests.go:110:9:110:9 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:108:15:108:78 | call to OpenFile | call to OpenFile |
-| tests.go:129:3:129:3 | f | tests.go:125:5:125:78 | ... := ...[0] | tests.go:129:3:129:3 | f | File handle may be writable as a result of data flow from a $@ and closing it may result in data loss upon failure, which is not handled explicitly. | tests.go:125:15:125:78 | call to OpenFile | call to OpenFile |

go/ql/test/query-tests/InconsistentCode/UnhandledCloseWritableHandle/UnhandledCloseWritableHandle.qlref

@@ -1 +1,2 @@
-InconsistentCode/UnhandledCloseWritableHandle.ql
+query: InconsistentCode/UnhandledCloseWritableHandle.ql
+postprocess: TestUtilities/PrettyPrintModels.ql