Skip to content

Commit 65fcd82

Browse files
Apostlex0Apostlex0
committed
code fixes
1 parent d881241 commit 65fcd82

2 files changed

Lines changed: 152 additions & 92 deletions

File tree

‎go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql‎

Lines changed: 134 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @name Missing MinVersion in tls.Config
33
* @id tob/go/missing-min-version-tls
4-
* @description Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients (default TLS 1.0) or Go < 1.22 for servers (default TLS 1.0). Does not mark explicitly set insecure versions.
4+
* @description Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients or Go < 1.22 for servers. Does not mark explicitly set versions (including explicitly insecure ones).
55
* @kind problem
66
* @tags security
77
* @problem.severity error
@@ -20,67 +20,59 @@ module TlsVersionConfig implements DataFlow::ConfigSig {
2020
/**
2121
* Holds if `source` is a TLS.Config instance.
2222
*/
23-
predicate isSource(DataFlow::Node source) {
24-
exists(Variable v |
25-
configOrConfigPointer(v.getType()) and
26-
source.asExpr() = v.getAReference()
27-
)
28-
}
29-
23+
predicate isSource(DataFlow::Node source) {
24+
exists(Variable v |
25+
configOrConfigPointer(v.getType()) and
26+
source.asExpr() = v.getAReference()
27+
)
28+
}
29+
3030
/**
3131
* Holds if a write to `sink`.MinVersion exists.
3232
*/
33-
predicate isSink(DataFlow::Node sink) {
34-
exists(Write fieldWrite, Field fld |
35-
fld.hasQualifiedName( "crypto/tls", "Config", "MinVersion") and
36-
fieldWrite.writesField(sink, fld, _)
37-
)
38-
}
33+
predicate isSink(DataFlow::Node sink) {
34+
exists(Write fieldWrite, Field fld |
35+
fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and
36+
fieldWrite.writesField(sink, fld, _)
37+
)
38+
}
3939
}
4040
module TlsVersionFlow = TaintTracking::Global<TlsVersionConfig>;
4141

42+
predicate structLitSetsMinVersion(StructLit lit) {
43+
exists(Write w, Field fld |
44+
fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and
45+
w.writesField(DataFlow::exprNode(lit), fld, _)
46+
)
47+
}
4248

43-
/**
44-
* Flow of a `tls.Config` with `MinVersion` to a variable.
45-
*/
4649
module TlsConfigCreationConfig implements DataFlow::ConfigSig {
47-
additional predicate isSecure(DataFlow::Node source) {
48-
exists(StructLit lit, Field fld |
49-
lit.getType().hasQualifiedName("crypto/tls", "Config") and
50-
fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and
51-
source.asExpr() = lit and
52-
fld = lit.getType().getField(_) and
53-
exists(Write w | w.writesField(DataFlow::exprNode(lit), fld, _))
54-
)
55-
}
56-
5750
/**
58-
* Holds if `source` is a TLS.Config literal.
51+
* Holds if `source` is a TLS.Config literal without MinVersion set.
5952
*/
60-
predicate isSource(DataFlow::Node source) {
61-
exists(StructLit lit, Field fld |
62-
lit.getType().hasQualifiedName("crypto/tls", "Config") and
63-
fld.hasQualifiedName("crypto/tls", "Config", "MinVersion") and
64-
source.asExpr() = lit
65-
)
66-
and not isSecure(source)
67-
}
68-
53+
predicate isSource(DataFlow::Node source) {
54+
exists(StructLit lit |
55+
lit.getType().hasQualifiedName("crypto/tls", "Config") and
56+
source.asExpr() = lit and
57+
not structLitSetsMinVersion(lit)
58+
)
59+
}
60+
6961
/**
7062
* Holds if it is TLS.Config instance (a Variable).
7163
*/
72-
predicate isSink(DataFlow::Node sink) {
64+
predicate isSink(DataFlow::Node sink) {
7365
exists(Variable v |
7466
sink.asExpr() = v.getAReference()
7567
)
76-
}
68+
}
7769

7870
/**
7971
* Holds if TLS.Config literal is saved in a structure's field
8072
*/
81-
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
82-
exists(Write w | w.writesField(succ, _, pred))
83-
}
73+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
74+
exists(Write w | w.writesField(succ, _, pred))
75+
}
8476
}
8577
module TlsConfigCreationFlow = TaintTracking::Global<TlsConfigCreationConfig>;
8678

@@ -108,6 +100,7 @@ predicate configOrConfigPointer(Type t) {
108100
* Holds if v is a Go version string for Go 1.x that is >= 1.18.
109101
* Matches: "1.18", "1.19", "1.20", ..., "1.99", "1.100", "1.18.0", etc.
110102
*/
103+
bindingset[v]
111104
predicate goVersionAtLeast_1_18(string v) {
112105
v.regexpMatch("1\\.(1[89]|[2-9][0-9]|[1-9][0-9]{2,})(\\.\\d+)?")
113106
}
@@ -116,6 +109,7 @@ predicate goVersionAtLeast_1_18(string v) {
116109
* Holds if v is a Go version string for Go 1.x that is >= 1.22.
117110
* Matches: "1.22", "1.23", ..., "1.99", "1.100", "1.22.0", etc.
118111
*/
112+
bindingset[v]
119113
predicate goVersionAtLeast_1_22(string v) {
120114
v.regexpMatch("1\\.(2[2-9]|[3-9][0-9]|[1-9][0-9]{2,})(\\.\\d+)?")
121115
}
@@ -124,12 +118,12 @@ predicate goVersionAtLeast_1_22(string v) {
124118
* Holds if the project may be built with a Go version where a server with
125119
* an unset MinVersion still defaults to TLS 1.0/1.1 (Go < 1.22).
126120
*
127-
* - If there is no go.mod: assume YES (be conservative).
128-
* - Otherwise: if ANY go.mod has a `go` directive < 1.22: YES.
121+
* - If there is no go.mod: assume yes
122+
* - Otherwise: if any go.mod has a `go` < 1.22: yes.
129123
*/
130124
predicate projectSupportsOldTlsDefaultsForServers() {
131-
not exists(GoMod::GoModGoLine _) or
132-
exists(GoMod::GoModGoLine l |
125+
not exists(GoModGoLine l | l = l) or
126+
exists(GoModGoLine l |
133127
not goVersionAtLeast_1_22(l.getVersion())
134128
)
135129
}
@@ -139,72 +133,136 @@ predicate projectSupportsOldTlsDefaultsForServers() {
139133
* an unset MinVersion still defaults to TLS 1.0/1.1 (Go < 1.18).
140134
*
141135
* - If there is no go.mod: assume YES (be conservative).
142-
* - Otherwise: if ANY go.mod has a `go` directive < 1.18: YES.
136+
* - Otherwise: if any go.mod has a `go` < 1.18: YES.
143137
*/
144138
predicate projectSupportsOldTlsDefaultsForClients() {
145-
not exists(GoMod::GoModGoLine _) or
146-
exists(GoMod::GoModGoLine l |
139+
not exists(GoModGoLine l | l = l) or
140+
exists(GoModGoLine l |
147141
not goVersionAtLeast_1_18(l.getVersion())
148142
)
149143
}
150144

151145
/**
152-
* Holds if the config is used as a client config (TLSClientConfig).
146+
* Holds if expression `e` mentions the config variable or struct literal.
153147
*/
154-
predicate isClientConfig(Variable v, StructLit configStruct) {
155-
exists(KeyValueExpr kv |
156-
kv.getKey().(VariableName).getTarget().getName() = "TLSClientConfig" and
157-
(
158-
kv.getValue().getAChild*() = v.getARead().asExpr()
159-
or
160-
kv.getValue().getAChild*() = configStruct
161-
)
162-
)
163-
or
164-
exists(Type t |
165-
t.hasQualifiedName("net/http", "Client") and
166-
v.getType() = t.getPointerType*()
167-
)
148+
predicate mentionsConfig(Expr e, Variable v, StructLit configStruct) {
149+
e.getAChild*() = v.getARead().asExpr() or
150+
e.getAChild*() = configStruct
151+
}
152+
153+
/**
154+
* Holds if the config is used as a client config (TLSClientConfig or tls.Dial/Client APIs).
155+
*/
156+
predicate usedAsClient(Variable v, StructLit configStruct) {
157+
exists(StructLit outer, KeyValueExpr kv |
158+
outer.getType().hasQualifiedName("net/http", "Transport") and
159+
kv.getParent*() = outer and
160+
kv.getKey().(Ident).getName() = "TLSClientConfig" and
161+
mentionsConfig(kv.getValue(), v, configStruct)
162+
)
163+
or
164+
exists(Write w, Field fld, DataFlow::Node recv, DataFlow::Node rhs |
165+
fld.hasQualifiedName("net/http", "Transport", "TLSClientConfig") and
166+
w.writesField(recv, fld, rhs) and
167+
mentionsConfig(rhs.asExpr(), v, configStruct)
168+
)
169+
or
170+
exists(CallExpr call |
171+
// tls.Dial(network, addr, config) => argument 2 is config (0-based)
172+
call.getTarget().hasQualifiedName("crypto/tls", "Dial") and
173+
mentionsConfig(call.getArgument(2), v, configStruct)
174+
)
175+
or
176+
exists(CallExpr call |
177+
// tls.DialWithDialer(dialer, network, addr, config) => argument 3 is config
178+
call.getTarget().hasQualifiedName("crypto/tls", "DialWithDialer") and
179+
mentionsConfig(call.getArgument(3), v, configStruct)
180+
)
181+
or
182+
exists(CallExpr call |
183+
// tls.Client(conn, config) => argument 1 is config
184+
call.getTarget().hasQualifiedName("crypto/tls", "Client") and
185+
mentionsConfig(call.getArgument(1), v, configStruct)
186+
)
187+
}
188+
189+
/**
190+
* Holds if the config is used as a server config (TLSConfig or tls.Listen/Server APIs).
191+
*/
192+
predicate usedAsServer(Variable v, StructLit configStruct) {
193+
exists(StructLit outer, KeyValueExpr kv |
194+
outer.getType().hasQualifiedName("net/http", "Server") and
195+
kv.getParent*() = outer and
196+
kv.getKey().(Ident).getName() = "TLSConfig" and
197+
mentionsConfig(kv.getValue(), v, configStruct)
198+
)
199+
or
200+
exists(Write w, Field fld, DataFlow::Node recv, DataFlow::Node rhs |
201+
fld.hasQualifiedName("net/http", "Server", "TLSConfig") and
202+
w.writesField(recv, fld, rhs) and
203+
mentionsConfig(rhs.asExpr(), v, configStruct)
204+
)
205+
or
206+
exists(CallExpr call |
207+
// tls.Listen(network, addr, config) => argument 2 is config
208+
call.getTarget().hasQualifiedName("crypto/tls", "Listen") and
209+
mentionsConfig(call.getArgument(2), v, configStruct)
210+
)
211+
or
212+
exists(CallExpr call |
213+
// tls.NewListener(inner, config) => argument 1 is config
214+
call.getTarget().hasQualifiedName("crypto/tls", "NewListener") and
215+
mentionsConfig(call.getArgument(1), v, configStruct)
216+
)
217+
or
218+
exists(CallExpr call |
219+
// tls.Server(conn, config) => argument 1 is config
220+
call.getTarget().hasQualifiedName("crypto/tls", "Server") and
221+
mentionsConfig(call.getArgument(1), v, configStruct)
222+
)
168223
}
169224

170225
// v - a variable holding any structure which is or contains the tls.Config
171226
from StructLit configStruct, Variable v, DataFlow::Node source, DataFlow::Node sink
172227
where
173228
// find tls.Config structures with MinVersion not set on the structure initialization
174229
(
175-
TlsConfigCreationFlow::flow(source, sink) and
176-
sink.asExpr() = v.getAReference() and
230+
TlsConfigCreationFlow::flow(source, sink) and
231+
sink.asExpr() = v.getAReference() and
177232
source.asExpr() = configStruct
178233
)
179234

180235
// only explicitely defined, e.g., skip function arguments
181236
and (
182237
exists(DeclStmt decl | v.getAReference() = decl.getAChild+())
183238
or
184-
exists(DefineStmt decl | v.getAReference() = decl.getAChild+())
239+
exists(DefineStmt decl | v.getAReference() = decl.getAChild+())
185240
)
186241

187242
// skip field declarations
188243
and not exists(FieldDecl decl | v.getAReference() = decl.getAChild+())
189-
244+
190245
// if the tls.Config is assigned to a variable
191246
and if configOrConfigPointer(v.getType()) then
192-
(
247+
(
193248
// exclude if there is a later write to MinVersion
194249
not exists(DataFlow::Node source2, DataFlow::Node sink2 |
195-
TlsVersionFlow::flow(source2, sink2) and
196-
source2.asExpr() = v.getAReference()
197-
)
250+
TlsVersionFlow::flow(source2, sink2) and
251+
source2.asExpr() = v.getAReference()
252+
)
198253
) else
199254
any()
200255

201256
// Version-aware filtering based on client vs server usage:
202257
// - For clients: only flag if Go < 1.18 (when client default is TLS 1.0)
203258
// - For servers: only flag if Go < 1.22 (when server default is TLS 1.0)
259+
// - If neither classified, be conservative as "server-like"
204260
and (
205-
(isClientConfig(v, configStruct) and projectSupportsOldTlsDefaultsForClients())
261+
(usedAsClient(v, configStruct) and projectSupportsOldTlsDefaultsForClients())
206262
or
207-
(not isClientConfig(v, configStruct) and projectSupportsOldTlsDefaultsForServers())
208-
)
263+
(usedAsServer(v, configStruct) and projectSupportsOldTlsDefaultsForServers())
264+
or
265+
(not usedAsClient(v, configStruct) and not usedAsServer(v, configStruct) and projectSupportsOldTlsDefaultsForServers())
266+
)
209267

210-
select configStruct, "TLS.Config.MinVersion is never set for variable $@ ", v, v.getName()
268+
select configStruct, "TLS.Config.MinVersion is never set for variable $@.", v, v.getName()

‎go/test/query-tests/security/MissingMinVersionTLS/MissingMinVersionTLS.expected‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
| MissingMinVersionTLS.go:25:14:25:25 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:25:3:25:8 | config | config |
2-
| MissingMinVersionTLS.go:35:14:37:3 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:35:3:35:8 | config | config |
3-
| MissingMinVersionTLS.go:50:13:50:24 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:50:3:50:8 | config | config |
4-
| MissingMinVersionTLS.go:61:13:61:24 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:61:3:61:8 | config | config |
5-
| MissingMinVersionTLS.go:91:12:91:23 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:110:3:110:3 | c | c |
6-
| MissingMinVersionTLS.go:103:12:105:2 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:118:3:118:3 | c | c |
7-
| MissingMinVersionTLS.go:103:12:105:2 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:149:3:149:3 | c | c |
8-
| MissingMinVersionTLS.go:126:23:126:62 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:126:3:126:3 | c | c |
9-
| MissingMinVersionTLS.go:135:10:135:49 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:135:3:135:5 | tmp | tmp |
10-
| MissingMinVersionTLS.go:135:10:135:49 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:136:3:136:3 | c | c |
11-
| MissingMinVersionTLS.go:142:23:142:62 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:142:3:142:3 | c | c |
12-
| MissingMinVersionTLS.go:149:23:149:62 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:149:3:149:3 | c | c |
13-
| MissingMinVersionTLS.go:159:21:161:5 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:157:3:157:8 | client | client |
14-
| MissingMinVersionTLS.go:168:16:168:55 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:167:3:167:5 | srv | srv |
15-
| MissingMinVersionTLS.go:171:16:171:55 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:170:3:170:6 | srv2 | srv2 |
16-
| MissingMinVersionTLS.go:176:9:176:48 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:182:3:182:6 | srv3 | srv3 |
1+
| MissingMinVersionTLS.go:25:14:25:25 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:25:3:25:8 | config | config |
2+
| MissingMinVersionTLS.go:35:14:37:3 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:35:3:35:8 | config | config |
3+
| MissingMinVersionTLS.go:50:13:50:24 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:50:3:50:8 | config | config |
4+
| MissingMinVersionTLS.go:61:13:61:24 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:61:3:61:8 | config | config |
5+
| MissingMinVersionTLS.go:91:12:91:23 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:110:3:110:3 | c | c |
6+
| MissingMinVersionTLS.go:103:12:105:2 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:118:3:118:3 | c | c |
7+
| MissingMinVersionTLS.go:103:12:105:2 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:149:3:149:3 | c | c |
8+
| MissingMinVersionTLS.go:126:23:126:62 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:126:3:126:3 | c | c |
9+
| MissingMinVersionTLS.go:135:10:135:49 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:135:3:135:5 | tmp | tmp |
10+
| MissingMinVersionTLS.go:135:10:135:49 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:136:3:136:3 | c | c |
11+
| MissingMinVersionTLS.go:142:23:142:62 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:142:3:142:3 | c | c |
12+
| MissingMinVersionTLS.go:149:23:149:62 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:149:3:149:3 | c | c |
13+
| MissingMinVersionTLS.go:159:23:161:5 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:157:3:157:8 | client | client |
14+
| MissingMinVersionTLS.go:169:16:169:55 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:168:3:168:5 | srv | srv |
15+
| MissingMinVersionTLS.go:172:16:172:55 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:171:3:171:6 | srv2 | srv2 |
16+
| MissingMinVersionTLS.go:177:9:177:48 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:177:3:177:3 | c | c |
17+
| MissingMinVersionTLS.go:177:9:177:48 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:178:3:178:8 | client | client |
18+
| MissingMinVersionTLS.go:177:9:177:48 | struct literal | TLS.Config.MinVersion is never set for variable $@. | MissingMinVersionTLS.go:183:3:183:6 | srv3 | srv3 |

0 commit comments

Comments
 (0)