Skip to content

Commit 6103749

Browse files
egregius313egregius313
authored
Merge pull request #16710 from egregius313/egregius313/go/dataflow/file-sources
Go: Add `file` sources
2 parents f0fe3a3 + b14c584 commit 6103749

File tree

14 files changed

+186
-42
lines changed

14 files changed

+186
-42
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* 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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,16 @@ extensions:
1010
- ["io/fs", "", False, "Sub", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
1111
- ["io/fs", "DirEntry", True, "Info", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
1212
- ["io/fs", "DirEntry", True, "Name", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
13+
- ["io/fs", "File", True, "Read", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
1314
- ["io/fs", "FS", True, "Open", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
1415
- ["io/fs", "GlobFS", True, "Glob", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
1516
- ["io/fs", "ReadDirFS", True, "ReadDir", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
1617
- ["io/fs", "ReadFileFS", True, "ReadFile", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
1718
- ["io/fs", "SubFS", True, "Sub", "", "", "Argument[receiver]", "ReturnValue[0]", "taint", "manual"]
19+
- addsTo:
20+
pack: codeql/go-all
21+
extensible: sourceModel
22+
data:
23+
- ["io/fs", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]
24+
- ["io/fs", "ReadFileFS", True, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]
25+
- ["io/fs", "FS", True, "Open", "", "", "ReturnValue[0]", "file", "manual"]

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,8 @@ extensions:
1414
data:
1515
- ["io/ioutil", "", False, "NopCloser", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
1616
- ["io/ioutil", "", False, "ReadAll", "", "", "Argument[0]", "ReturnValue[0]", "taint", "manual"]
17+
- addsTo:
18+
pack: codeql/go-all
19+
extensible: sourceModel
20+
data:
21+
- ["io/ioutil", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,12 @@ extensions:
4040
- ["os", "", False, "ExpandEnv", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
4141
- ["os", "", False, "NewFile", "", "", "Argument[0]", "ReturnValue", "taint", "manual"]
4242
- ["os", "File", True, "Fd", "", "", "Argument[receiver]", "ReturnValue", "taint", "manual"]
43+
- ["os", "File", True, "Read", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
44+
- ["os", "File", True, "ReadAt", "", "", "Argument[receiver]", "Argument[0]", "taint", "manual"]
45+
- addsTo:
46+
pack: codeql/go-all
47+
extensible: sourceModel
48+
data:
49+
- ["os", "", False, "Open", "", "", "ReturnValue[0]", "file", "manual"]
50+
- ["os", "", False, "OpenFile", "", "", "ReturnValue[0]", "file", "manual"]
51+
- ["os", "", False, "ReadFile", "", "", "ReturnValue[0]", "file", "manual"]

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
models
2-
| 1 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
3-
| 2 | Source: net/http; Request; true; Body; ; ; ; remote; manual |
2+
| 1 | Summary: io/fs; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
3+
| 2 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
4+
| 3 | Source: net/http; Request; true; Body; ; ; ; remote; manual |
5+
| 4 | Summary: os; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
46
edges
57
| Builtin.go:6:2:6:2 | definition of b | Builtin.go:8:9:8:17 | type conversion | provenance | |
6-
| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
8+
| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
9+
| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
10+
| Builtin.go:7:2:7:15 | selection of Body | Builtin.go:6:2:6:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
711
| Builtin.go:12:2:12:2 | definition of b | Builtin.go:17:9:17:17 | type conversion | provenance | |
8-
| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
12+
| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
13+
| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
14+
| Builtin.go:13:2:13:15 | selection of Body | Builtin.go:12:2:12:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
915
| Builtin.go:21:2:21:2 | definition of b | Builtin.go:24:10:24:18 | type conversion | provenance | |
10-
| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:2 MaD:1 |
16+
| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:1 |
17+
| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:2 |
18+
| Builtin.go:22:2:22:15 | selection of Body | Builtin.go:21:2:21:2 | definition of b | provenance | Src:MaD:3 MaD:4 |
1119
nodes
1220
| Builtin.go:6:2:6:2 | definition of b | semmle.label | definition of b |
1321
| 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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
testFailures
2+
invalidModelRow
3+
failures

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/threat-models
4+
extensible: threatModelConfiguration
5+
data:
6+
- ["file", true, 0]

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package test
2+
3+
import (
4+
"io/fs"
5+
"io/ioutil"
6+
"os"
7+
)
8+
9+
func open() {
10+
file, err := os.Open("file.txt") // $ source
11+
if err != nil {
12+
return
13+
}
14+
defer file.Close()
15+
file.Read([]byte{1, 2, 3})
16+
}
17+
18+
func openFile() {
19+
file, err := os.OpenFile("file.txt", os.O_RDWR, 0) // $source
20+
if err != nil {
21+
return
22+
}
23+
defer file.Close()
24+
file.Read([]byte{1, 2, 3})
25+
}
26+
27+
func readFile() {
28+
data, err := os.ReadFile("file.txt") // $source
29+
if err != nil {
30+
return
31+
}
32+
_ = data
33+
}
34+
35+
func readFileIoUtil() {
36+
data, err := ioutil.ReadFile("file.txt") // $source
37+
if err != nil {
38+
return
39+
}
40+
_ = data
41+
}
42+
43+
func getFileFS() fs.ReadFileFS {
44+
return nil
45+
}
46+
47+
func readFileFs() {
48+
data, err := fs.ReadFile(os.DirFS("."), "file.txt") // $source
49+
if err != nil {
50+
return
51+
}
52+
_ = data
53+
54+
dir := getFileFS()
55+
data, err = dir.ReadFile("file.txt") // $source
56+
57+
if err != nil {
58+
return
59+
}
60+
_ = data
61+
}
62+
63+
func fsOpen() {
64+
file, err := os.DirFS(".").Open("file.txt") // $source
65+
if err != nil {
66+
return
67+
}
68+
defer file.Close()
69+
file.Read([]byte{1, 2, 3})
70+
}

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import go
2+
import ModelValidation
3+
import TestUtilities.InlineExpectationsTest
4+
5+
module SourceTest implements TestSig {
6+
string getARelevantTag() { result = "source" }
7+
8+
predicate hasActualResult(Location location, string element, string tag, string value) {
9+
exists(ThreatModelFlowSource s |
10+
s.hasLocationInfo(location.getFile().getAbsolutePath(), location.getStartLine(),
11+
location.getStartColumn(), location.getEndLine(), location.getEndColumn()) and
12+
element = s.toString() and
13+
value = "" and
14+
tag = "source"
15+
)
16+
}
17+
}
18+
19+
import MakeTest<SourceTest>

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,19 @@ edges
2828
| test.go:51:2:51:30 | ... := ...[0] | test.go:52:16:52:37 | index expression | provenance | Src:MaD:10 |
2929
| test.go:57:2:57:46 | ... := ...[0] | test.go:58:13:58:22 | fileHeader | provenance | Src:MaD:11 |
3030
| test.go:58:2:58:29 | ... := ...[0] | test.go:60:2:60:5 | file | provenance | |
31-
| test.go:58:13:58:22 | fileHeader | test.go:58:2:58:29 | ... := ...[0] | provenance | MaD:16 |
31+
| test.go:58:13:58:22 | fileHeader | test.go:58:2:58:29 | ... := ...[0] | provenance | MaD:17 |
3232
| test.go:59:2:59:7 | definition of buffer | test.go:61:20:61:25 | buffer | provenance | |
3333
| test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:15 |
34+
| test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:16 |
35+
| test.go:60:2:60:5 | file | test.go:59:2:59:7 | definition of buffer | provenance | MaD:18 |
3436
| test.go:66:2:66:31 | ... := ...[0] | test.go:67:16:67:41 | index expression | provenance | Src:MaD:12 |
3537
| test.go:72:2:72:31 | ... := ...[0] | test.go:74:13:74:22 | fileHeader | provenance | Src:MaD:12 |
3638
| test.go:74:2:74:29 | ... := ...[0] | test.go:76:2:76:5 | file | provenance | |
37-
| test.go:74:13:74:22 | fileHeader | test.go:74:2:74:29 | ... := ...[0] | provenance | MaD:16 |
39+
| test.go:74:13:74:22 | fileHeader | test.go:74:2:74:29 | ... := ...[0] | provenance | MaD:17 |
3840
| test.go:75:2:75:7 | definition of buffer | test.go:77:20:77:25 | buffer | provenance | |
3941
| test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:15 |
42+
| test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:16 |
43+
| test.go:76:2:76:5 | file | test.go:75:2:75:7 | definition of buffer | provenance | MaD:18 |
4044
| test.go:82:2:82:32 | ... := ...[0] | test.go:83:16:83:24 | selection of Value | provenance | Src:MaD:13 |
4145
| test.go:88:13:88:25 | call to Cookies | test.go:89:16:89:31 | selection of Value | provenance | Src:MaD:14 |
4246
| test.go:99:11:99:15 | &... | test.go:100:16:100:21 | selection of s | provenance | Src:MaD:3 |
@@ -49,7 +53,7 @@ edges
4953
| test.go:136:11:136:32 | call to Param | test.go:137:29:137:41 | type conversion | provenance | Src:MaD:4 |
5054
| test.go:148:11:148:32 | call to Param | test.go:149:30:149:34 | param | provenance | Src:MaD:4 |
5155
| test.go:149:12:149:35 | call to NewReader | test.go:150:31:150:36 | reader | provenance | |
52-
| test.go:149:30:149:34 | param | test.go:149:12:149:35 | call to NewReader | provenance | MaD:17 |
56+
| test.go:149:30:149:34 | param | test.go:149:12:149:35 | call to NewReader | provenance | MaD:19 |
5357
| test.go:164:11:164:32 | call to Param | test.go:165:23:165:35 | type conversion | provenance | Src:MaD:4 |
5458
models
5559
| 1 | Summary: github.com/labstack/echo; Context; true; Get; ; ; Argument[receiver]; ReturnValue; taint; manual |
@@ -66,9 +70,11 @@ models
6670
| 12 | Source: github.com/labstack/echo; Context; true; MultipartForm; ; ; ReturnValue[0]; remote; manual |
6771
| 13 | Source: github.com/labstack/echo; Context; true; Cookie; ; ; ReturnValue[0]; remote; manual |
6872
| 14 | Source: github.com/labstack/echo; Context; true; Cookies; ; ; ReturnValue[0]; remote; manual |
69-
| 15 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
70-
| 16 | Summary: mime/multipart; FileHeader; true; Open; ; ; Argument[receiver]; ReturnValue[0]; taint; manual |
71-
| 17 | Summary: strings; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual |
73+
| 15 | Summary: io/fs; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
74+
| 16 | Summary: io; Reader; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
75+
| 17 | Summary: mime/multipart; FileHeader; true; Open; ; ; Argument[receiver]; ReturnValue[0]; taint; manual |
76+
| 18 | Summary: os; File; true; Read; ; ; Argument[receiver]; Argument[0]; taint; manual |
77+
| 19 | Summary: strings; ; false; NewReader; ; ; Argument[0]; ReturnValue; taint; manual |
7278
nodes
7379
| test.go:15:11:15:32 | call to Param | semmle.label | call to Param |
7480
| test.go:16:16:16:20 | param | semmle.label | param |

0 commit comments

Comments
 (0)