Skip to content

Commit a7382e0

Browse files
owen-mcowen-mc
committed
Make ClearTextLogging use new API
The extra nodes in .expected files are due to the changes from #13717, which are not applied to configuration classes extending DataFlow::Configuration or TaintTracking::Configuration.
1 parent 653563f commit a7382e0

3 files changed

Lines changed: 88 additions & 19 deletions

File tree

‎go/ql/lib/semmle/go/security/CleartextLogging.qll‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ module CleartextLogging {
1717
import CleartextLoggingCustomizations::CleartextLogging
1818

1919
/**
20+
* DEPRECATED: Use `Flow` instead.
21+
*
2022
* A data-flow tracking configuration for clear-text logging of sensitive information.
2123
*
2224
* This configuration identifies flows from `Source`s, which are sources of
@@ -25,7 +27,7 @@ module CleartextLogging {
2527
* added either by extending the relevant class, or by subclassing this configuration itself,
2628
* and amending the sources and sinks.
2729
*/
28-
class Configuration extends DataFlow::Configuration {
30+
deprecated class Configuration extends DataFlow::Configuration {
2931
Configuration() { this = "CleartextLogging" }
3032

3133
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -56,4 +58,36 @@ module CleartextLogging {
5658
not any(Protobuf::GetMethod gm).taintStep(src, trg)
5759
}
5860
}
61+
62+
private module Config implements DataFlow::ConfigSig {
63+
predicate isSource(DataFlow::Node source) { source instanceof Source }
64+
65+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
66+
67+
predicate isBarrier(DataFlow::Node node) {
68+
node instanceof Barrier
69+
or
70+
exists(DataFlow::CallNode call | node = call.getResult() |
71+
call.getTarget() = Builtin::error().getType().getMethod("Error")
72+
or
73+
call.getTarget().(Method).hasQualifiedName("fmt", "Stringer", "String")
74+
)
75+
}
76+
77+
predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
78+
// A taint propagating data-flow edge through structs: a tainted write taints the entire struct.
79+
exists(Write write |
80+
write.writesField(trg.(DataFlow::PostUpdateNode).getPreUpdateNode(), _, src)
81+
)
82+
or
83+
// taint steps that do not include flow through fields. Field reads would produce FPs due to
84+
// the additional taint step above that taints whole structs from individual field writes.
85+
TaintTracking::localTaintStep(src, trg) and
86+
not TaintTracking::fieldReadStep(src, trg) and
87+
// Also exclude protobuf field fetches, since they amount to single field reads.
88+
not any(Protobuf::GetMethod gm).taintStep(src, trg)
89+
}
90+
}
91+
92+
module Flow = DataFlow::Global<Config>;
5993
}

‎go/ql/src/Security/CWE-312/CleartextLogging.ql‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
*/
1515

1616
import go
17-
import semmle.go.security.CleartextLogging::CleartextLogging
18-
import DataFlow::PathGraph
17+
import semmle.go.security.CleartextLogging
18+
import CleartextLogging::Flow::PathGraph
1919

20-
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
21-
where cfg.hasFlowPath(source, sink)
20+
from CleartextLogging::Flow::PathNode source, CleartextLogging::Flow::PathNode sink
21+
where CleartextLogging::Flow::flowPath(source, sink)
2222
select sink.getNode(), source, sink, "$@ flows to a logging call.", source.getNode(),
23-
"Sensitive data returned by " + source.getNode().(Source).describe()
23+
"Sensitive data returned by " + source.getNode().(CleartextLogging::Source).describe()

‎go/ql/test/query-tests/Security/CWE-312/CleartextLogging.expected‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,46 @@
11
edges
2-
| klog.go:20:30:20:37 | selection of Header | klog.go:22:15:22:20 | header |
2+
| klog.go:20:3:25:3 | range statement[1] | klog.go:20:13:20:19 | definition of headers |
3+
| klog.go:20:13:20:19 | definition of headers | klog.go:21:27:21:33 | headers |
4+
| klog.go:20:30:20:37 | selection of Header | klog.go:20:3:25:3 | range statement[1] |
5+
| klog.go:21:4:24:4 | range statement[1] | klog.go:21:11:21:16 | definition of header |
6+
| klog.go:21:11:21:16 | definition of header | klog.go:22:15:22:20 | header |
7+
| klog.go:21:27:21:33 | headers | klog.go:21:4:24:4 | range statement[1] |
38
| klog.go:28:13:28:20 | selection of Header | klog.go:28:13:28:41 | call to Get |
4-
| main.go:21:19:21:26 | password | main.go:22:29:22:34 | fields |
9+
| main.go:20:2:20:7 | definition of fields | main.go:22:29:22:34 | fields |
10+
| main.go:21:19:21:26 | password | main.go:20:2:20:7 | definition of fields |
511
| overrides.go:9:9:9:16 | password | overrides.go:13:14:13:23 | call to String |
612
| passwords.go:8:12:8:12 | definition of x | passwords.go:9:14:9:14 | x |
713
| passwords.go:30:8:30:15 | password | passwords.go:8:12:8:12 | definition of x |
814
| passwords.go:34:28:34:35 | password | passwords.go:34:14:34:35 | ...+... |
9-
| passwords.go:37:13:37:13 | x | passwords.go:39:14:39:17 | obj1 |
10-
| passwords.go:42:6:42:13 | password | passwords.go:44:14:44:17 | obj2 |
11-
| passwords.go:48:11:48:18 | password | passwords.go:47:14:47:17 | obj3 |
12-
| passwords.go:86:16:86:36 | call to make | passwords.go:88:14:88:26 | utilityObject |
13-
| passwords.go:90:12:90:19 | password | passwords.go:91:23:91:28 | secret |
15+
| passwords.go:36:2:36:5 | definition of obj1 | passwords.go:39:14:39:17 | obj1 |
16+
| passwords.go:36:10:38:2 | struct literal | passwords.go:36:2:36:5 | definition of obj1 |
17+
| passwords.go:37:13:37:13 | x | passwords.go:36:10:38:2 | struct literal |
18+
| passwords.go:41:2:41:5 | definition of obj2 | passwords.go:44:14:44:17 | obj2 |
19+
| passwords.go:41:10:43:2 | struct literal | passwords.go:41:2:41:5 | definition of obj2 |
20+
| passwords.go:42:6:42:13 | password | passwords.go:41:10:43:2 | struct literal |
21+
| passwords.go:46:6:46:9 | definition of obj3 | passwords.go:47:14:47:17 | obj3 |
22+
| passwords.go:48:11:48:18 | password | passwords.go:46:6:46:9 | definition of obj3 |
23+
| passwords.go:85:2:85:14 | definition of utilityObject | passwords.go:88:14:88:26 | utilityObject |
24+
| passwords.go:85:19:87:2 | struct literal | passwords.go:85:2:85:14 | definition of utilityObject |
25+
| passwords.go:86:16:86:36 | call to make | passwords.go:85:19:87:2 | struct literal |
26+
| passwords.go:90:2:90:7 | definition of secret | passwords.go:91:23:91:28 | secret |
27+
| passwords.go:90:12:90:19 | password | passwords.go:90:2:90:7 | definition of secret |
1428
| passwords.go:101:33:101:40 | password | passwords.go:101:15:101:40 | ...+... |
1529
| passwords.go:107:34:107:41 | password | passwords.go:107:16:107:41 | ...+... |
1630
| passwords.go:112:33:112:40 | password | passwords.go:112:15:112:40 | ...+... |
17-
| passwords.go:116:28:116:36 | password1 | passwords.go:116:14:116:45 | ...+... |
1831
| passwords.go:116:28:116:36 | password1 | passwords.go:116:28:116:45 | call to String |
1932
| passwords.go:116:28:116:45 | call to String | passwords.go:116:14:116:45 | ...+... |
20-
| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:126:14:126:19 | config [x] |
21-
| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:127:14:127:19 | config [y] |
22-
| passwords.go:119:13:119:13 | x | passwords.go:125:14:125:19 | config |
33+
| passwords.go:118:2:118:7 | definition of config | passwords.go:125:14:125:19 | config |
34+
| passwords.go:118:2:118:7 | definition of config [x] | passwords.go:126:14:126:19 | config [x] |
35+
| passwords.go:118:2:118:7 | definition of config [y] | passwords.go:127:14:127:19 | config [y] |
36+
| passwords.go:118:12:123:2 | struct literal | passwords.go:118:2:118:7 | definition of config |
37+
| passwords.go:118:12:123:2 | struct literal [x] | passwords.go:118:2:118:7 | definition of config [x] |
38+
| passwords.go:118:12:123:2 | struct literal [y] | passwords.go:118:2:118:7 | definition of config [y] |
39+
| passwords.go:119:13:119:13 | x | passwords.go:118:12:123:2 | struct literal |
40+
| passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal |
2341
| passwords.go:121:13:121:20 | password | passwords.go:118:12:123:2 | struct literal [x] |
24-
| passwords.go:121:13:121:20 | password | passwords.go:125:14:125:19 | config |
42+
| passwords.go:122:13:122:25 | call to getPassword | passwords.go:118:12:123:2 | struct literal |
2543
| passwords.go:122:13:122:25 | call to getPassword | passwords.go:118:12:123:2 | struct literal [y] |
26-
| passwords.go:122:13:122:25 | call to getPassword | passwords.go:125:14:125:19 | config |
2744
| passwords.go:126:14:126:19 | config [x] | passwords.go:126:14:126:21 | selection of x |
2845
| passwords.go:127:14:127:19 | config [y] | passwords.go:127:14:127:21 | selection of y |
2946
| protobuf.go:11:2:11:6 | definition of query [pointer, Description] | protobuf.go:12:2:12:6 | query [pointer, Description] |
@@ -38,13 +55,19 @@ edges
3855
| protos/query/query.pb.go:119:10:119:10 | x [pointer, Description] | protos/query/query.pb.go:119:10:119:10 | implicit dereference [Description] |
3956
| util.go:16:9:16:18 | selection of password | passwords.go:28:14:28:28 | call to getPassword |
4057
nodes
58+
| klog.go:20:3:25:3 | range statement[1] | semmle.label | range statement[1] |
59+
| klog.go:20:13:20:19 | definition of headers | semmle.label | definition of headers |
4160
| klog.go:20:30:20:37 | selection of Header | semmle.label | selection of Header |
61+
| klog.go:21:4:24:4 | range statement[1] | semmle.label | range statement[1] |
62+
| klog.go:21:11:21:16 | definition of header | semmle.label | definition of header |
63+
| klog.go:21:27:21:33 | headers | semmle.label | headers |
4264
| klog.go:22:15:22:20 | header | semmle.label | header |
4365
| klog.go:28:13:28:20 | selection of Header | semmle.label | selection of Header |
4466
| klog.go:28:13:28:41 | call to Get | semmle.label | call to Get |
4567
| main.go:15:14:15:21 | password | semmle.label | password |
4668
| main.go:17:12:17:19 | password | semmle.label | password |
4769
| main.go:18:17:18:24 | password | semmle.label | password |
70+
| main.go:20:2:20:7 | definition of fields | semmle.label | definition of fields |
4871
| main.go:21:19:21:26 | password | semmle.label | password |
4972
| main.go:22:29:22:34 | fields | semmle.label | fields |
5073
| main.go:25:35:25:42 | password | semmle.label | password |
@@ -60,15 +83,23 @@ nodes
6083
| passwords.go:32:12:32:19 | password | semmle.label | password |
6184
| passwords.go:34:14:34:35 | ...+... | semmle.label | ...+... |
6285
| passwords.go:34:28:34:35 | password | semmle.label | password |
86+
| passwords.go:36:2:36:5 | definition of obj1 | semmle.label | definition of obj1 |
87+
| passwords.go:36:10:38:2 | struct literal | semmle.label | struct literal |
6388
| passwords.go:37:13:37:13 | x | semmle.label | x |
6489
| passwords.go:39:14:39:17 | obj1 | semmle.label | obj1 |
90+
| passwords.go:41:2:41:5 | definition of obj2 | semmle.label | definition of obj2 |
91+
| passwords.go:41:10:43:2 | struct literal | semmle.label | struct literal |
6592
| passwords.go:42:6:42:13 | password | semmle.label | password |
6693
| passwords.go:44:14:44:17 | obj2 | semmle.label | obj2 |
94+
| passwords.go:46:6:46:9 | definition of obj3 | semmle.label | definition of obj3 |
6795
| passwords.go:47:14:47:17 | obj3 | semmle.label | obj3 |
6896
| passwords.go:48:11:48:18 | password | semmle.label | password |
6997
| passwords.go:51:14:51:27 | fixed_password | semmle.label | fixed_password |
98+
| passwords.go:85:2:85:14 | definition of utilityObject | semmle.label | definition of utilityObject |
99+
| passwords.go:85:19:87:2 | struct literal | semmle.label | struct literal |
70100
| passwords.go:86:16:86:36 | call to make | semmle.label | call to make |
71101
| passwords.go:88:14:88:26 | utilityObject | semmle.label | utilityObject |
102+
| passwords.go:90:2:90:7 | definition of secret | semmle.label | definition of secret |
72103
| passwords.go:90:12:90:19 | password | semmle.label | password |
73104
| passwords.go:91:23:91:28 | secret | semmle.label | secret |
74105
| passwords.go:101:15:101:40 | ...+... | semmle.label | ...+... |
@@ -80,6 +111,10 @@ nodes
80111
| passwords.go:116:14:116:45 | ...+... | semmle.label | ...+... |
81112
| passwords.go:116:28:116:36 | password1 | semmle.label | password1 |
82113
| passwords.go:116:28:116:45 | call to String | semmle.label | call to String |
114+
| passwords.go:118:2:118:7 | definition of config | semmle.label | definition of config |
115+
| passwords.go:118:2:118:7 | definition of config [x] | semmle.label | definition of config [x] |
116+
| passwords.go:118:2:118:7 | definition of config [y] | semmle.label | definition of config [y] |
117+
| passwords.go:118:12:123:2 | struct literal | semmle.label | struct literal |
83118
| passwords.go:118:12:123:2 | struct literal [x] | semmle.label | struct literal [x] |
84119
| passwords.go:118:12:123:2 | struct literal [y] | semmle.label | struct literal [y] |
85120
| passwords.go:119:13:119:13 | x | semmle.label | x |

0 commit comments

Comments
 (0)