Skip to content

Commit b795469

Browse files
krishnprakashkrishnprakash
authored
Merge branch 'github:main' into main-1
2 parents 88c576f + 087c555 commit b795469

File tree

1,159 files changed

+21145
-13178
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

1,159 files changed

+21145
-13178
lines changed

‎go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql

+1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ predicate isCloseSink(DataFlow::Node sink, DataFlow::CallNode closeCall) {
9797
// where the function is called on the sink
9898
closeCall.getReceiver() = sink and
9999
// and check that it is not dominated by a call to `os.File.Sync`.
100+
// TODO: fix this logic when `closeCall` is in a defer statement.
100101
not exists(IR::Instruction syncInstr, DataFlow::Node syncReceiver, DataFlow::CallNode syncCall |
101102
// match the instruction corresponding to an `os.File.Sync` call with the predecessor
102103
syncCall.asInstruction() = syncInstr and

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

+46-42
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,52 @@
11
#select
2-
| 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 |
3-
| 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 |
4-
| 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 |
5-
| 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 |
6-
| 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 |
7-
| 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 |
8-
| 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 |
9-
| 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 |
2+
| tests.go:10:8:10:8 | f | tests.go:32:5:32:78 | ... := ...[0] | tests.go:10:8:10: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:32:15:32:78 | call to OpenFile | call to OpenFile |
3+
| tests.go:10:8:10:8 | f | tests.go:46:5:46:76 | ... := ...[0] | tests.go:10:8:10: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:46:15:46:76 | call to OpenFile | call to OpenFile |
4+
| tests.go:15:3:15:3 | f | tests.go:32:5:32:78 | ... := ...[0] | tests.go:15:3:15: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:32:15:32:78 | call to OpenFile | call to OpenFile |
5+
| tests.go:15:3:15:3 | f | tests.go:46:5:46:76 | ... := ...[0] | tests.go:15:3:15: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:46:15:46:76 | call to OpenFile | call to OpenFile |
6+
| tests.go:57:3:57:3 | f | tests.go:55:5:55:78 | ... := ...[0] | tests.go:57:3:57: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:55:15:55:78 | call to OpenFile | call to OpenFile |
7+
| tests.go:69:3:69:3 | f | tests.go:67:5:67:76 | ... := ...[0] | tests.go:69:3:69: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:67:15:67:76 | call to OpenFile | call to OpenFile |
8+
| tests.go:111:9:111:9 | f | tests.go:109:5:109:78 | ... := ...[0] | tests.go:111:9:111: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:109:15:109:78 | call to OpenFile | call to OpenFile |
9+
| tests.go:130:3:130:3 | f | tests.go:126:5:126:78 | ... := ...[0] | tests.go:130:3:130: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:126:15:126:78 | call to OpenFile | call to OpenFile |
10+
| tests.go:151:8:151:8 | f | tests.go:147:2:147:74 | ... := ...[0] | tests.go:151:8:151: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:147:12:147:74 | call to OpenFile | call to OpenFile |
1011
edges
11-
| tests.go:8:24:8:24 | definition of f | tests.go:9:8:9:8 | f | provenance | |
12-
| tests.go:12:32:12:32 | definition of f | tests.go:13:13:15:2 | capture variable f | provenance | |
13-
| tests.go:13:13:15:2 | capture variable f | tests.go:14:3:14:3 | f | provenance | |
14-
| tests.go:31:5:31:78 | ... := ...[0] | tests.go:32:21:32:21 | f | provenance | Src:MaD:1 |
15-
| tests.go:31:5:31:78 | ... := ...[0] | tests.go:33:29:33:29 | f | provenance | Src:MaD:1 |
16-
| tests.go:32:21:32:21 | f | tests.go:8:24:8:24 | definition of f | provenance | |
17-
| tests.go:33:29:33:29 | f | tests.go:12:32:12:32 | definition of f | provenance | |
18-
| tests.go:45:5:45:76 | ... := ...[0] | tests.go:46:21:46:21 | f | provenance | Src:MaD:1 |
19-
| tests.go:45:5:45:76 | ... := ...[0] | tests.go:47:29:47:29 | f | provenance | Src:MaD:1 |
20-
| tests.go:46:21:46:21 | f | tests.go:8:24:8:24 | definition of f | provenance | |
21-
| tests.go:47:29:47:29 | f | tests.go:12:32:12:32 | definition of f | provenance | |
22-
| tests.go:54:5:54:78 | ... := ...[0] | tests.go:56:3:56:3 | f | provenance | Src:MaD:1 |
23-
| tests.go:66:5:66:76 | ... := ...[0] | tests.go:68:3:68:3 | f | provenance | Src:MaD:1 |
24-
| tests.go:108:5:108:78 | ... := ...[0] | tests.go:110:9:110:9 | f | provenance | Src:MaD:1 |
25-
| tests.go:125:5:125:78 | ... := ...[0] | tests.go:129:3:129:3 | f | provenance | Src:MaD:1 |
12+
| tests.go:9:24:9:24 | definition of f | tests.go:10:8:10:8 | f | provenance | |
13+
| tests.go:13:32:13:32 | definition of f | tests.go:14:13:16:2 | capture variable f | provenance | |
14+
| tests.go:14:13:16:2 | capture variable f | tests.go:15:3:15:3 | f | provenance | |
15+
| tests.go:32:5:32:78 | ... := ...[0] | tests.go:33:21:33:21 | f | provenance | Src:MaD:1 |
16+
| tests.go:32:5:32:78 | ... := ...[0] | tests.go:34:29:34:29 | f | provenance | Src:MaD:1 |
17+
| tests.go:33:21:33:21 | f | tests.go:9:24:9:24 | definition of f | provenance | |
18+
| tests.go:34:29:34:29 | f | tests.go:13:32:13:32 | definition of f | provenance | |
19+
| tests.go:46:5:46:76 | ... := ...[0] | tests.go:47:21:47:21 | f | provenance | Src:MaD:1 |
20+
| tests.go:46:5:46:76 | ... := ...[0] | tests.go:48:29:48:29 | f | provenance | Src:MaD:1 |
21+
| tests.go:47:21:47:21 | f | tests.go:9:24:9:24 | definition of f | provenance | |
22+
| tests.go:48:29:48:29 | f | tests.go:13:32:13:32 | definition of f | provenance | |
23+
| tests.go:55:5:55:78 | ... := ...[0] | tests.go:57:3:57:3 | f | provenance | Src:MaD:1 |
24+
| tests.go:67:5:67:76 | ... := ...[0] | tests.go:69:3:69:3 | f | provenance | Src:MaD:1 |
25+
| tests.go:109:5:109:78 | ... := ...[0] | tests.go:111:9:111:9 | f | provenance | Src:MaD:1 |
26+
| tests.go:126:5:126:78 | ... := ...[0] | tests.go:130:3:130:3 | f | provenance | Src:MaD:1 |
27+
| tests.go:147:2:147:74 | ... := ...[0] | tests.go:151:8:151:8 | f | provenance | Src:MaD:1 |
2628
models
2729
| 1 | Source: os; ; false; OpenFile; ; ; ReturnValue[0]; file; manual |
2830
nodes
29-
| tests.go:8:24:8:24 | definition of f | semmle.label | definition of f |
30-
| tests.go:9:8:9:8 | f | semmle.label | f |
31-
| tests.go:12:32:12:32 | definition of f | semmle.label | definition of f |
32-
| tests.go:13:13:15:2 | capture variable f | semmle.label | capture variable f |
33-
| tests.go:14:3:14:3 | f | semmle.label | f |
34-
| tests.go:31:5:31:78 | ... := ...[0] | semmle.label | ... := ...[0] |
35-
| tests.go:32:21:32:21 | f | semmle.label | f |
36-
| tests.go:33:29:33:29 | f | semmle.label | f |
37-
| tests.go:45:5:45:76 | ... := ...[0] | semmle.label | ... := ...[0] |
38-
| tests.go:46:21:46:21 | f | semmle.label | f |
39-
| tests.go:47:29:47:29 | f | semmle.label | f |
40-
| tests.go:54:5:54:78 | ... := ...[0] | semmle.label | ... := ...[0] |
41-
| tests.go:56:3:56:3 | f | semmle.label | f |
42-
| tests.go:66:5:66:76 | ... := ...[0] | semmle.label | ... := ...[0] |
43-
| tests.go:68:3:68:3 | f | semmle.label | f |
44-
| tests.go:108:5:108:78 | ... := ...[0] | semmle.label | ... := ...[0] |
45-
| tests.go:110:9:110:9 | f | semmle.label | f |
46-
| tests.go:125:5:125:78 | ... := ...[0] | semmle.label | ... := ...[0] |
47-
| tests.go:129:3:129:3 | f | semmle.label | f |
31+
| tests.go:9:24:9:24 | definition of f | semmle.label | definition of f |
32+
| tests.go:10:8:10:8 | f | semmle.label | f |
33+
| tests.go:13:32:13:32 | definition of f | semmle.label | definition of f |
34+
| tests.go:14:13:16:2 | capture variable f | semmle.label | capture variable f |
35+
| tests.go:15:3:15:3 | f | semmle.label | f |
36+
| tests.go:32:5:32:78 | ... := ...[0] | semmle.label | ... := ...[0] |
37+
| tests.go:33:21:33:21 | f | semmle.label | f |
38+
| tests.go:34:29:34:29 | f | semmle.label | f |
39+
| tests.go:46:5:46:76 | ... := ...[0] | semmle.label | ... := ...[0] |
40+
| tests.go:47:21:47:21 | f | semmle.label | f |
41+
| tests.go:48:29:48:29 | f | semmle.label | f |
42+
| tests.go:55:5:55:78 | ... := ...[0] | semmle.label | ... := ...[0] |
43+
| tests.go:57:3:57:3 | f | semmle.label | f |
44+
| tests.go:67:5:67:76 | ... := ...[0] | semmle.label | ... := ...[0] |
45+
| tests.go:69:3:69:3 | f | semmle.label | f |
46+
| tests.go:109:5:109:78 | ... := ...[0] | semmle.label | ... := ...[0] |
47+
| tests.go:111:9:111:9 | f | semmle.label | f |
48+
| tests.go:126:5:126:78 | ... := ...[0] | semmle.label | ... := ...[0] |
49+
| tests.go:130:3:130:3 | f | semmle.label | f |
50+
| tests.go:147:2:147:74 | ... := ...[0] | semmle.label | ... := ...[0] |
51+
| tests.go:151:8:151:8 | f | semmle.label | f |
4852
subpaths

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
query: InconsistentCode/UnhandledCloseWritableHandle.ql
2-
postprocess: utils/test/PrettyPrintModels.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql

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

+39-12
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
package test
22

33
import (
4+
"io"
45
"log"
56
"os"
67
)
78

89
func closeFileDeferred(f *os.File) {
9-
defer f.Close() // NOT OK, if `f` is writable
10+
defer f.Close() // $ Alert=w Alert=rw
1011
}
1112

1213
func closeFileDeferredIndirect(f *os.File) {
1314
var cont = func() {
14-
f.Close() // NOT OK, if `f` is writable
15+
f.Close() // $ Alert=w Alert=rw
1516
}
1617

1718
defer cont()
@@ -28,7 +29,7 @@ func closeFileDeferredIndirectReturn(f *os.File) {
2829

2930
func deferredCalls() {
3031
// open file for writing
31-
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
32+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source=w
3233
closeFileDeferred(f) // NOT OK
3334
closeFileDeferredIndirect(f) // NOT OK
3435
closeFileDeferredIndirectReturn(f) // OK - the error is not discarded at the call to Close (though it is discarded later)
@@ -42,7 +43,7 @@ func deferredCalls() {
4243
}
4344

4445
// open file for reading and writing
45-
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
46+
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source=rw
4647
closeFileDeferred(f) // NOT OK
4748
closeFileDeferredIndirect(f) // NOT OK
4849
closeFileDeferredIndirectReturn(f) // OK - the error is not discarded at the call to Close (though it is discarded later)
@@ -51,9 +52,9 @@ func deferredCalls() {
5152

5253
func notDeferred() {
5354
// open file for writing
54-
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
55+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source
5556
// the handle is write-only and we don't check if `Close` succeeds
56-
f.Close() // NOT OK
57+
f.Close() // $ Alert
5758
}
5859

5960
// open file for reading
@@ -63,9 +64,9 @@ func notDeferred() {
6364
}
6465

6566
// open file for reading and writing
66-
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
67+
if f, err := os.OpenFile("foo.txt", os.O_RDWR|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source
6768
// the handle is read-write and we don't check if `Close` succeeds
68-
f.Close() // NOT OK
69+
f.Close() // $ Alert
6970
}
7071
}
7172

@@ -105,9 +106,9 @@ func deferredCloseWithSync() {
105106

106107
func deferredCloseWithSyncEarlyReturn(n int) {
107108
// open file for writing
108-
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
109+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source
109110
// a call to `Close` is deferred
110-
defer f.Close() // NOT OK
111+
defer f.Close() // $ Alert
111112

112113
if n > 100 {
113114
return
@@ -122,10 +123,36 @@ func deferredCloseWithSyncEarlyReturn(n int) {
122123

123124
func unhandledSync() {
124125
// open file for writing
125-
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil {
126+
if f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666); err != nil { // $ Source
126127
// we have a call to `Sync` which precedes the call to `Close`, but there is no check
127128
// to see if `Sync` may have failed
128129
f.Sync()
129-
f.Close() // NOT OK
130+
f.Close() // $ Alert
131+
}
132+
}
133+
134+
func returnedSync() error {
135+
// open file for writing
136+
f, err := os.OpenFile("foo.txt", os.O_WRONLY|os.O_TRUNC|os.O_CREATE, 0666)
137+
if err != nil {
138+
// we have a call to `Sync` which precedes the call to `Close`, but there is no check
139+
// to see if `Sync` may have failed
140+
return err
141+
}
142+
defer f.Close()
143+
return f.Sync()
144+
}
145+
146+
func copyFile(destFile string, mode os.FileMode, src io.Reader) error {
147+
f, err := os.OpenFile(destFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, mode) // $ Source
148+
if err != nil {
149+
return err
150+
}
151+
defer f.Close() // $ SPURIOUS: Alert
152+
153+
_, err = io.Copy(f, src)
154+
if err != nil {
155+
return err
130156
}
157+
return f.Sync()
131158
}

0 commit comments

Comments
 (0)