Skip to content

Commit 746290d

Browse files
alexrfordalexrford
authored
Merge pull request #7713 from github/ruby/clear-text-logging
Ruby: Add `rb/clear-text-logging-sensitive-data` query
2 parents 1d437dd + dd383f9 commit 746290d

File tree

12 files changed

+735
-1
lines changed

12 files changed

+735
-1
lines changed

‎config/identical-files.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,8 @@
465465
],
466466
"SensitiveDataHeuristics Python/JS": [
467467
"javascript/ql/lib/semmle/javascript/security/internal/SensitiveDataHeuristics.qll",
468-
"python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll"
468+
"python/ql/lib/semmle/python/security/internal/SensitiveDataHeuristics.qll",
469+
"ruby/ql/lib/codeql/ruby/security/internal/SensitiveDataHeuristics.qll"
469470
],
470471
"ReDoS Util Python/JS/Ruby": [
471472
"javascript/ql/lib/semmle/javascript/security/performance/ReDoSUtil.qll",

‎ruby/ql/lib/codeql/ruby/security/CleartextLoggingCustomizations.qll

Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,303 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about
3+
* cleartext logging of sensitive information, as well as extension points for
4+
* adding your own.
5+
*/
6+
7+
private import ruby
8+
private import codeql.ruby.DataFlow
9+
private import codeql.ruby.TaintTracking::TaintTracking
10+
private import codeql.ruby.Concepts
11+
private import codeql.ruby.dataflow.RemoteFlowSources
12+
private import internal.SensitiveDataHeuristics::HeuristicNames
13+
private import codeql.ruby.CFG
14+
private import codeql.ruby.dataflow.SSA
15+
16+
/**
17+
* Provides default sources, sinks and sanitizers for reasoning about
18+
* cleartext logging of sensitive information, as well as extension points for
19+
* adding your own.
20+
*/
21+
module CleartextLogging {
22+
/**
23+
* A data flow source for cleartext logging of sensitive information.
24+
*/
25+
abstract class Source extends DataFlow::Node {
26+
/** Gets a string that describes the type of this data flow source. */
27+
abstract string describe();
28+
}
29+
30+
/**
31+
* A data flow sink for cleartext logging of sensitive information.
32+
*/
33+
abstract class Sink extends DataFlow::Node { }
34+
35+
/**
36+
* A sanitizer for cleartext logging of sensitive information.
37+
*/
38+
abstract class Sanitizer extends DataFlow::Node { }
39+
40+
/**
41+
* Holds if `re` may be a regular expression that can be used to sanitize
42+
* sensitive data with a call to `sub`.
43+
*/
44+
private predicate effectiveSubRegExp(CfgNodes::ExprNodes::RegExpLiteralCfgNode re) {
45+
re.getConstantValue().getStringOrSymbol().matches([".*", ".+"])
46+
}
47+
48+
/**
49+
* Holds if `re` may be a regular expression that can be used to sanitize
50+
* sensitive data with a call to `gsub`.
51+
*/
52+
private predicate effectiveGsubRegExp(CfgNodes::ExprNodes::RegExpLiteralCfgNode re) {
53+
re.getConstantValue().getStringOrSymbol().matches(".")
54+
}
55+
56+
/**
57+
* A call to `sub`/`sub!` or `gsub`/`gsub!` that seems to mask sensitive information.
58+
*/
59+
private class MaskingReplacerSanitizer extends Sanitizer, DataFlow::CallNode {
60+
MaskingReplacerSanitizer() {
61+
exists(CfgNodes::ExprNodes::RegExpLiteralCfgNode re |
62+
re = this.getArgument(0).asExpr() and
63+
(
64+
this.getMethodName() = ["sub", "sub!"] and effectiveSubRegExp(re)
65+
or
66+
this.getMethodName() = ["gsub", "gsub!"] and effectiveGsubRegExp(re)
67+
)
68+
)
69+
}
70+
}
71+
72+
/**
73+
* Like `MaskingReplacerSanitizer` but updates the receiver for methods that
74+
* sanitize the receiver.
75+
* Taint is thereby cleared for any subsequent read.
76+
*/
77+
private class InPlaceMaskingReplacerSanitizer extends Sanitizer {
78+
InPlaceMaskingReplacerSanitizer() {
79+
exists(MaskingReplacerSanitizer m | m.getMethodName() = ["gsub!", "sub!"] |
80+
m.getReceiver() = this
81+
)
82+
}
83+
}
84+
85+
/**
86+
* Holds if `name` is for a method or variable that appears, syntactically, to
87+
* not be sensitive.
88+
*/
89+
bindingset[name]
90+
private predicate nameIsNotSensitive(string name) {
91+
name.regexpMatch(notSensitiveRegexp()) and
92+
// By default `notSensitiveRegexp()` includes some false positives for
93+
// common ruby method names that are not necessarily non-sensitive.
94+
// We explicitly exclude element references, element assignments, and
95+
// mutation methods.
96+
not name = ["[]", "[]="] and
97+
not name.matches("%!")
98+
}
99+
100+
/**
101+
* A call that might obfuscate a password, for example through hashing.
102+
*/
103+
private class ObfuscatorCall extends Sanitizer, DataFlow::CallNode {
104+
ObfuscatorCall() { nameIsNotSensitive(this.getMethodName()) }
105+
}
106+
107+
/**
108+
* A data flow node that does not contain a clear-text password, according to its syntactic name.
109+
*/
110+
private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
111+
NameGuidedNonCleartextPassword() {
112+
exists(string name | nameIsNotSensitive(name) |
113+
// accessing a non-sensitive variable
114+
this.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = name
115+
or
116+
// dereferencing a non-sensitive field
117+
this.asExpr()
118+
.(CfgNodes::ExprNodes::ElementReferenceCfgNode)
119+
.getArgument(0)
120+
.getConstantValue()
121+
.getStringOrSymbol() = name
122+
or
123+
// calling a non-sensitive method
124+
this.(DataFlow::CallNode).getMethodName() = name
125+
)
126+
or
127+
// avoid i18n strings
128+
this.asExpr()
129+
.(CfgNodes::ExprNodes::ElementReferenceCfgNode)
130+
.getReceiver()
131+
.getConstantValue()
132+
.getStringOrSymbol()
133+
.regexpMatch("(?is).*(messages|strings).*")
134+
}
135+
}
136+
137+
/**
138+
* A data flow node that receives flow that is not a clear-text password.
139+
*/
140+
private class NonCleartextPasswordFlow extends NonCleartextPassword {
141+
NonCleartextPasswordFlow() {
142+
any(NonCleartextPassword other).(DataFlow::LocalSourceNode).flowsTo(this)
143+
}
144+
}
145+
146+
/**
147+
* A data flow node that does not contain a clear-text password.
148+
*/
149+
abstract private class NonCleartextPassword extends DataFlow::Node { }
150+
151+
// `writeNode` assigns pair with key `name` to `val`
152+
private predicate hashKeyWrite(DataFlow::CallNode writeNode, string name, DataFlow::Node val) {
153+
writeNode.asExpr().getExpr() instanceof SetterMethodCall and
154+
// hash[name]
155+
writeNode.getArgument(0).asExpr().getConstantValue().getStringOrSymbol() = name and
156+
// val
157+
writeNode.getArgument(1).asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs() =
158+
val.asExpr()
159+
}
160+
161+
/**
162+
* A write to a hash entry with a value that may contain password information.
163+
*/
164+
private class HashKeyWritePasswordSource extends Source {
165+
private string name;
166+
private DataFlow::ExprNode recv;
167+
168+
HashKeyWritePasswordSource() {
169+
exists(DataFlow::Node val |
170+
name.regexpMatch(maybePassword()) and
171+
not nameIsNotSensitive(name) and
172+
// avoid safe values assigned to presumably unsafe names
173+
not val instanceof NonCleartextPassword and
174+
(
175+
// hash[name] = val
176+
hashKeyWrite(this, name, val) and
177+
recv = this.(DataFlow::CallNode).getReceiver()
178+
)
179+
)
180+
}
181+
182+
override string describe() { result = "a write to " + name }
183+
184+
/** Gets the name of the key */
185+
string getName() { result = name }
186+
187+
/**
188+
* Gets the name of the hash variable that this password source is assigned
189+
* to, if applicable.
190+
*/
191+
LocalVariable getVariable() {
192+
result = recv.getExprNode().getExpr().(VariableReadAccess).getVariable()
193+
}
194+
}
195+
196+
/**
197+
* A hash literal with an entry that may contain a password
198+
*/
199+
private class HashLiteralPasswordSource extends Source {
200+
private string name;
201+
202+
HashLiteralPasswordSource() {
203+
exists(DataFlow::Node val, CfgNodes::ExprNodes::HashLiteralCfgNode lit |
204+
name.regexpMatch(maybePassword()) and
205+
not name.regexpMatch(notSensitiveRegexp()) and
206+
// avoid safe values assigned to presumably unsafe names
207+
not val instanceof NonCleartextPassword and
208+
// hash = { name: val }
209+
exists(CfgNodes::ExprNodes::PairCfgNode p |
210+
this.asExpr() = lit and p = lit.getAKeyValuePair()
211+
|
212+
p.getKey().getConstantValue().getStringOrSymbol() = name and
213+
p.getValue() = val.asExpr()
214+
)
215+
)
216+
}
217+
218+
override string describe() { result = "an write to " + name }
219+
}
220+
221+
/** An assignment that may assign a password to a variable */
222+
private class AssignPasswordVariableSource extends Source {
223+
string name;
224+
225+
AssignPasswordVariableSource() {
226+
// avoid safe values assigned to presumably unsafe names
227+
not this instanceof NonCleartextPassword and
228+
name.regexpMatch(maybePassword()) and
229+
exists(Assignment a |
230+
this.asExpr().getExpr() = a.getRightOperand() and
231+
a.getLeftOperand().getAVariable().getName() = name
232+
)
233+
}
234+
235+
override string describe() { result = "an assignment to " + name }
236+
}
237+
238+
/** A parameter that may contain a password. */
239+
private class ParameterPasswordSource extends Source {
240+
private string name;
241+
242+
ParameterPasswordSource() {
243+
name.regexpMatch(maybePassword()) and
244+
not this instanceof NonCleartextPassword and
245+
exists(Parameter p, LocalVariable v |
246+
v = p.getAVariable() and
247+
v.getName() = name and
248+
this.asExpr().getExpr() = v.getAnAccess()
249+
)
250+
}
251+
252+
override string describe() { result = "a parameter " + name }
253+
}
254+
255+
/** A call that might return a password. */
256+
private class CallPasswordSource extends DataFlow::CallNode, Source {
257+
private string name;
258+
259+
CallPasswordSource() {
260+
name = this.getMethodName() and
261+
name.regexpMatch("(?is)getPassword")
262+
}
263+
264+
override string describe() { result = "a call to " + name }
265+
}
266+
267+
private string commonLogMethodName() {
268+
result = ["info", "debug", "warn", "warning", "error", "log"]
269+
}
270+
271+
/** Holds if `nodeFrom` taints `nodeTo`. */
272+
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
273+
exists(string name, ElementReference ref, LocalVariable hashVar |
274+
// from `hsh[password] = "changeme"` to a `hsh[password]` read
275+
nodeFrom.(HashKeyWritePasswordSource).getName() = name and
276+
nodeTo.asExpr().getExpr() = ref and
277+
ref.getArgument(0).getConstantValue().getStringOrSymbol() = name and
278+
nodeFrom.(HashKeyWritePasswordSource).getVariable() = hashVar and
279+
ref.getReceiver().(VariableReadAccess).getVariable() = hashVar and
280+
nodeFrom.asExpr().getASuccessor*() = nodeTo.asExpr()
281+
)
282+
}
283+
284+
/**
285+
* A node representing an expression whose value is logged.
286+
*/
287+
private class LoggingInputAsSink extends Sink {
288+
LoggingInputAsSink() {
289+
// precise match based on inferred type of receiver
290+
exists(Logging logging | this = logging.getAnInput())
291+
or
292+
// imprecise name based match
293+
exists(DataFlow::CallNode call, string recvName |
294+
recvName =
295+
call.getReceiver().asExpr().getExpr().(VariableReadAccess).getVariable().getName() and
296+
recvName.regexpMatch(".*log(ger)?") and
297+
call.getMethodName() = commonLogMethodName()
298+
|
299+
this = call.getArgument(_)
300+
)
301+
}
302+
}
303+
}

‎ruby/ql/lib/codeql/ruby/security/CleartextLoggingQuery.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Provides a taint-tracking configuration for "Clear-text logging of sensitive information".
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `CleartextLogging::Configuration` is needed, otherwise
6+
* `CleartextLoggingCustomizations` should be imported instead.
7+
*/
8+
9+
private import ruby
10+
private import codeql.ruby.DataFlow
11+
private import codeql.ruby.TaintTracking
12+
import CleartextLoggingCustomizations::CleartextLogging
13+
private import CleartextLoggingCustomizations::CleartextLogging as CleartextLogging
14+
15+
/**
16+
* A taint-tracking configuration for detecting "Clear-text logging of sensitive information".
17+
*/
18+
class Configuration extends TaintTracking::Configuration {
19+
Configuration() { this = "CleartextLogging" }
20+
21+
override predicate isSource(DataFlow::Node source) { source instanceof CleartextLogging::Source }
22+
23+
override predicate isSink(DataFlow::Node sink) { sink instanceof CleartextLogging::Sink }
24+
25+
override predicate isSanitizer(DataFlow::Node node) {
26+
super.isSanitizer(node)
27+
or
28+
node instanceof CleartextLogging::Sanitizer
29+
}
30+
31+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
32+
CleartextLogging::isAdditionalTaintStep(nodeFrom, nodeTo)
33+
}
34+
}

0 commit comments

Comments
 (0)