Skip to content

Commit 91ccd30

Browse files
alexrfordalexrford
committed
Ruby: Implement rb/clear-text-logging-sensitive-data
1 parent cfb2d7f commit 91ccd30

File tree

6 files changed

+474
-0
lines changed

6 files changed

+474
-0
lines changed

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

Lines changed: 323 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,323 @@
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+
module CleartextLogging {
17+
/**
18+
* A data flow source for cleartext logging of sensitive information.
19+
*/
20+
abstract class Source extends DataFlow::Node {
21+
/** Gets a string that describes the type of this data flow source. */
22+
abstract string describe();
23+
}
24+
25+
/**
26+
* A data flow sink for cleartext logging of sensitive information.
27+
*/
28+
abstract class Sink extends DataFlow::Node { }
29+
30+
/**
31+
* A sanitizer for cleartext logging of sensitive information.
32+
*/
33+
abstract class Sanitizer extends DataFlow::Node { }
34+
35+
/**
36+
* A node that receives sanitized sensitive information.
37+
*/
38+
abstract class SanitizerIn 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(RegExpLiteral 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(RegExpLiteral 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(RegExpLiteral re |
62+
re = this.getArgument(0).asExpr().getExpr() 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+
* A node sanitized by a prior call to `sub!` or `gsub!`,
74+
* e.g. the `password` argument to `info` in:
75+
* ```
76+
* password = "changeme"
77+
* password.sub!(/.+/, "")
78+
* Logger.new(STDOUT).info password
79+
* ```
80+
*/
81+
private class MaskingReplacerSanitizedNode extends SanitizerIn {
82+
MaskingReplacerSanitizedNode() {
83+
exists(MaskingReplacerSanitizer maskCall, Variable v |
84+
maskCall.getMethodName() = ["sub!", "gsub!"] and
85+
v = maskCall.getReceiver().asExpr().getExpr().(VariableReadAccess).getVariable() and
86+
v = this.asExpr().getExpr().(VariableReadAccess).getVariable() and
87+
maskCall.asExpr().getASuccessor*() = this.asExpr()
88+
)
89+
}
90+
}
91+
92+
/**
93+
* Gets the name of a method that would be falsely marked as non-sensitive
94+
* by `notSensitiveRegexp`.
95+
*/
96+
private predicate nonSensitiveMethodNameExclusion(string name) {
97+
name = ["[]", "[]="]
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() {
105+
this.getMethodName().regexpMatch(notSensitiveRegexp()) and
106+
not nonSensitiveMethodNameExclusion(this.getMethodName())
107+
}
108+
}
109+
110+
/**
111+
* A data flow node that does not contain a clear-text password, according to its syntactic name.
112+
*/
113+
private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
114+
NameGuidedNonCleartextPassword() {
115+
exists(string name | name.regexpMatch(notSensitiveRegexp()) |
116+
// accessing a non-sensitive variable
117+
this.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = name
118+
or
119+
// dereferencing a non-sensitive field
120+
this.asExpr()
121+
.getExpr()
122+
.(ElementReference)
123+
.getArgument(0)
124+
.getConstantValue()
125+
.getStringOrSymbol() = name
126+
or
127+
// calling a non-sensitive method
128+
(
129+
this.(DataFlow::CallNode).getMethodName() = name and
130+
not nonSensitiveMethodNameExclusion(name)
131+
)
132+
)
133+
or
134+
// avoid i18n strings
135+
this.asExpr()
136+
.getExpr()
137+
.(ElementReference)
138+
.getReceiver()
139+
.getConstantValue()
140+
.getStringOrSymbol()
141+
.regexpMatch("(?is).*(messages|strings).*")
142+
}
143+
}
144+
145+
/**
146+
* A data flow node that receives flow that is not a clear-text password.
147+
*/
148+
private class NonCleartextPasswordFlow extends NonCleartextPassword {
149+
NonCleartextPasswordFlow() {
150+
any(NonCleartextPassword other).(DataFlow::LocalSourceNode).flowsTo(this)
151+
}
152+
}
153+
154+
/**
155+
* A data flow node that does not contain a clear-text password.
156+
*/
157+
abstract private class NonCleartextPassword extends DataFlow::Node { }
158+
159+
// `writeNode` assigns pair with key `name` to `val`
160+
private predicate hashKeyWrite(DataFlow::CallNode writeNode, string name, DataFlow::Node val) {
161+
exists(SetterMethodCall setter |
162+
setter = writeNode.asExpr().getExpr() and
163+
// hash[name]
164+
setter.getArgument(0).getConstantValue().getStringOrSymbol() = name and
165+
// val
166+
setter.getArgument(1).(Assignment).getRightOperand() = val.asExpr().getExpr()
167+
)
168+
}
169+
170+
/**
171+
* An hash with a value that may contain password information
172+
*
173+
* This is a source since logging a hash will show the pairs present.
174+
*/
175+
private abstract class HashPasswordSource extends Source {
176+
/** Gets the name of the key */
177+
abstract string getName();
178+
179+
/**
180+
* Gets the name of the hash variable that this password source is assigned
181+
* to, if applicable.
182+
*/
183+
abstract LocalVariable getVariable();
184+
}
185+
186+
private class HashKeyWritePasswordSource extends HashPasswordSource {
187+
private string name;
188+
private DataFlow::ExprNode recv;
189+
190+
HashKeyWritePasswordSource() {
191+
exists(DataFlow::Node val |
192+
name.regexpMatch(maybePassword()) and
193+
not name.regexpMatch(notSensitiveRegexp()) and
194+
// avoid safe values assigned to presumably unsafe names
195+
not val instanceof NonCleartextPassword and
196+
(
197+
// hash[name] = val
198+
hashKeyWrite(this, name, val) and
199+
recv = this.(DataFlow::CallNode).getReceiver()
200+
)
201+
)
202+
}
203+
204+
override string describe() { result = "an write to " + name }
205+
override string getName() { result = name }
206+
override LocalVariable getVariable() {
207+
result = recv.getExprNode().getExpr().(VariableReadAccess).getVariable()
208+
}
209+
}
210+
211+
private class HashLiteralPasswordSource extends HashPasswordSource {
212+
private string name;
213+
private HashLiteral lit;
214+
215+
HashLiteralPasswordSource() {
216+
exists(DataFlow::Node val |
217+
name.regexpMatch(maybePassword()) and
218+
not name.regexpMatch(notSensitiveRegexp()) and
219+
// avoid safe values assigned to presumably unsafe names
220+
not val instanceof NonCleartextPassword and
221+
// hash = { name: val }
222+
exists(Pair p |
223+
this.asExpr().getExpr() = lit and p = lit.getAKeyValuePair() |
224+
p.getKey().getConstantValue().getStringOrSymbol() = name and
225+
p.getValue() = val.asExpr().getExpr()
226+
)
227+
)
228+
}
229+
230+
override string describe() { result = "an write to " + name }
231+
override string getName() { result = name }
232+
override LocalVariable getVariable() {
233+
exists(Assignment a |
234+
a.getRightOperand() = lit |
235+
result = a.getLeftOperand().getAVariable()
236+
)
237+
}
238+
}
239+
240+
/** An assignment that may assign a password to a variable */
241+
private class AssignPasswordVariableSource extends Source {
242+
string name;
243+
244+
AssignPasswordVariableSource() {
245+
// avoid safe values assigned to presumably unsafe names
246+
not this instanceof NonCleartextPassword and
247+
name.regexpMatch(maybePassword()) and
248+
(
249+
exists(Assignment a |
250+
this.asExpr().getExpr() = a.getRightOperand() and
251+
a.getLeftOperand().getAVariable().getName() = name)
252+
)
253+
}
254+
255+
override string describe() { result = "an assignment to " + name }
256+
}
257+
258+
/** A parameter that may contain a password. */
259+
private class ParameterPasswordSource extends Source {
260+
private string name;
261+
262+
ParameterPasswordSource() {
263+
name.regexpMatch(maybePassword()) and
264+
not this instanceof NonCleartextPassword and
265+
exists(Parameter p, LocalVariable v |
266+
v = p.getAVariable() and
267+
v.getName() = name and
268+
this.asExpr().getExpr() = v.getAnAccess()
269+
)
270+
}
271+
272+
override string describe() { result = "a parameter " + name }
273+
}
274+
275+
/** A call that might return a password. */
276+
private class CallPasswordSource extends DataFlow::CallNode, Source {
277+
private string name;
278+
279+
CallPasswordSource() {
280+
name = this.getMethodName() and
281+
name.regexpMatch("(?is)getPassword")
282+
}
283+
284+
override string describe() { result = "a call to " + name }
285+
}
286+
287+
private string commonLogMethodName() {
288+
result = ["info", "debug", "warn", "warning", "error", "log"]
289+
}
290+
291+
/** Holds if `nodeFrom` taints `nodeTo`. */
292+
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
293+
exists(string name, ElementReference ref, LocalVariable hashVar |
294+
// from `hsh[password] = "changeme"` to a `hsh[password]` read
295+
nodeFrom.(HashKeyWritePasswordSource).getName() = name and
296+
nodeTo.asExpr().getExpr() = ref and
297+
ref.getArgument(0).getConstantValue().getStringOrSymbol() = name and
298+
nodeFrom.(HashPasswordSource).getVariable() = hashVar and
299+
ref.getReceiver().(VariableReadAccess).getVariable() = hashVar and
300+
nodeFrom.asExpr().getASuccessor*() = nodeTo.asExpr()
301+
)
302+
}
303+
304+
/**
305+
* A node representing an expression whose value is logged.
306+
*/
307+
private class LoggingInputAsSink extends Sink {
308+
LoggingInputAsSink() {
309+
// precise match based on inferred type of receiver
310+
exists(Logging logging | this = logging.getAnInput())
311+
or
312+
// imprecise name based match
313+
exists(DataFlow::CallNode call, string recvName |
314+
recvName =
315+
call.getReceiver().asExpr().getExpr().(VariableReadAccess).getVariable().getName() and
316+
recvName.regexpMatch(".*log(ger)?") and
317+
call.getMethodName() = commonLogMethodName()
318+
|
319+
this = call.getArgument(_)
320+
)
321+
}
322+
}
323+
}

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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 isSanitizerIn(DataFlow::Node node) {
32+
node instanceof CleartextLogging::SanitizerIn
33+
}
34+
35+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
36+
CleartextLogging::isAdditionalTaintStep(nodeFrom, nodeTo)
37+
}
38+
}

‎ruby/ql/src/queries/security/cwe-312/CleartextLogging.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Clear-text logging of sensitive information
3+
* @description Logging sensitive information without encryption or hashing can
4+
* expose it to an attacker.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id rb/clear-text-logging-sensitive-data
10+
* @tags security
11+
* external/cwe/cwe-312
12+
* external/cwe/cwe-359
13+
* external/cwe/cwe-532
14+
*/
15+
16+
import ruby
17+
import codeql.ruby.security.CleartextLoggingQuery
18+
import codeql.ruby.DataFlow
19+
import DataFlow::PathGraph
20+
21+
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
22+
where config.hasFlowPath(source, sink)
23+
select sink.getNode(), source, sink,
24+
"Sensitive data returned by $@ is logged here.", source.getNode(), source.getNode().(Source).describe()

‎ruby/ql/test/query-tests/security/cwe-312/CleartextLogging.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
edges
2+
nodes
3+
subpaths
4+
#select

‎ruby/ql/test/query-tests/security/cwe-312/CleartextLogging.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/security/cwe-312/CleartextLogging.ql

0 commit comments

Comments
 (0)