Skip to content

Commit 8b51ee8

Browse files
Use additional sensitive data heuristics in CleartextSources
1 parent a756f86 commit 8b51ee8

File tree

1 file changed

+61
-45
lines changed

1 file changed

+61
-45
lines changed

ruby/ql/lib/codeql/ruby/security/internal/CleartextSources.qll

Lines changed: 61 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.ruby.DataFlow
99
private import codeql.ruby.TaintTracking::TaintTracking
1010
private import codeql.ruby.dataflow.RemoteFlowSources
1111
private import SensitiveDataHeuristics::HeuristicNames
12+
private import SensitiveDataHeuristics
1213
private import codeql.ruby.CFG
1314
private import codeql.ruby.dataflow.SSA
1415

@@ -92,17 +93,17 @@ module CleartextSources {
9293
}
9394

9495
/**
95-
* A call that might obfuscate a password, for example through hashing.
96+
* A call that might obfuscate sensitive data, for example through hashing.
9697
*/
9798
private class ObfuscatorCall extends Sanitizer, DataFlow::CallNode {
9899
ObfuscatorCall() { nameIsNotSensitive(this.getMethodName()) }
99100
}
100101

101102
/**
102-
* A data flow node that does not contain a clear-text password, according to its syntactic name.
103+
* A data flow node that does not contain clear-text sensitive data, according to its syntactic name.
103104
*/
104-
private class NameGuidedNonCleartextPassword extends NonCleartextPassword {
105-
NameGuidedNonCleartextPassword() {
105+
private class NameGuidedNonCleartextSensitive extends NonCleartextSensitive {
106+
NameGuidedNonCleartextSensitive() {
106107
exists(string name | nameIsNotSensitive(name) |
107108
// accessing a non-sensitive variable
108109
this.asExpr().getExpr().(VariableReadAccess).getVariable().getName() = name
@@ -129,18 +130,23 @@ module CleartextSources {
129130
}
130131

131132
/**
132-
* A data flow node that receives flow that is not a clear-text password.
133+
* A data flow node that receives flow that is not clear-text sensitive data.
133134
*/
134-
class NonCleartextPasswordFlow extends NonCleartextPassword {
135-
NonCleartextPasswordFlow() {
136-
any(NonCleartextPassword other).(DataFlow::LocalSourceNode).flowsTo(this)
135+
class NonCleartextSensitiveFlow extends NonCleartextSensitive {
136+
NonCleartextSensitiveFlow() {
137+
any(NonCleartextSensitive other).(DataFlow::LocalSourceNode).flowsTo(this)
137138
}
138139
}
139140

140141
/**
141-
* A data flow node that does not contain a clear-text password.
142+
* DEPRECATED: Use NonCleartextSensitiveFlow instead.
142143
*/
143-
abstract private class NonCleartextPassword extends DataFlow::Node { }
144+
deprecated class NonCleartextPasswordFlow = NonCleartextSensitiveFlow;
145+
146+
/**
147+
* A data flow node that does not contain clear-text sensitive data.
148+
*/
149+
abstract private class NonCleartextSensitive extends DataFlow::Node { }
144150

145151
// `writeNode` assigns pair with key `name` to `val`
146152
private predicate hashKeyWrite(DataFlow::CallNode writeNode, string name, DataFlow::Node val) {
@@ -153,18 +159,19 @@ module CleartextSources {
153159
}
154160

155161
/**
156-
* A value written to a hash entry with a key that may contain password information.
162+
* A value written to a hash entry with a key that may contain sensitive information.
157163
*/
158-
private class HashKeyWritePasswordSource extends Source {
164+
private class HashKeyWriteSensitiveSource extends Source {
159165
private string name;
160166
private DataFlow::ExprNode recv;
161167

162-
HashKeyWritePasswordSource() {
163-
exists(DataFlow::CallNode writeNode |
164-
name.regexpMatch(maybePassword()) and
168+
HashKeyWriteSensitiveSource() {
169+
exists(DataFlow::CallNode writeNode, SensitiveDataClassification classification |
170+
nameIndicatesSensitiveData(name, classification) and
171+
not classification = SensitiveDataClassification::id() and
165172
not nameIsNotSensitive(name) and
166173
// avoid safe values assigned to presumably unsafe names
167-
not this instanceof NonCleartextPassword and
174+
not this instanceof NonCleartextSensitive and
168175
// hash[name] = val
169176
hashKeyWrite(writeNode, name, this) and
170177
recv = writeNode.getReceiver()
@@ -177,7 +184,7 @@ module CleartextSources {
177184
string getName() { result = name }
178185

179186
/**
180-
* Gets the name of the hash variable that this password source is assigned
187+
* Gets the name of the hash variable that this sensitive source is assigned
181188
* to, if applicable.
182189
*/
183190
LocalVariable getVariable() {
@@ -186,17 +193,20 @@ module CleartextSources {
186193
}
187194

188195
/**
189-
* An entry into a hash literal that may contain a password
196+
* An entry into a hash literal that may contain sensitive data
190197
*/
191-
private class HashLiteralPasswordSource extends Source {
198+
private class HashLiteralSensitiveSource extends Source {
192199
private string name;
193200

194-
HashLiteralPasswordSource() {
195-
exists(CfgNodes::ExprNodes::HashLiteralCfgNode lit |
196-
name.regexpMatch(maybePassword()) and
201+
HashLiteralSensitiveSource() {
202+
exists(
203+
CfgNodes::ExprNodes::HashLiteralCfgNode lit, SensitiveDataClassification classification
204+
|
205+
nameIndicatesSensitiveData(name, classification) and
206+
not classification = SensitiveDataClassification::id() and
197207
not nameIsNotSensitive(name) and
198208
// avoid safe values assigned to presumably unsafe names
199-
not this instanceof NonCleartextPassword and
209+
not this instanceof NonCleartextSensitive and
200210
// hash = { name: val }
201211
exists(CfgNodes::ExprNodes::PairCfgNode p | p = lit.getAKeyValuePair() |
202212
p.getKey().getConstantValue().getStringlikeValue() = name and
@@ -208,36 +218,42 @@ module CleartextSources {
208218
override string describe() { result = "a write to " + name }
209219
}
210220

211-
/** An assignment that may assign a password to a variable */
212-
private class AssignPasswordVariableSource extends Source {
221+
/** An assignment that may assign sensitive data to a variable */
222+
private class AssignSensitiveVariableSource extends Source {
213223
string name;
214224

215-
AssignPasswordVariableSource() {
216-
// avoid safe values assigned to presumably unsafe names
217-
not this instanceof NonCleartextPassword and
218-
name.regexpMatch(maybePassword()) and
219-
not nameIsNotSensitive(name) and
220-
exists(Assignment a |
221-
this.asExpr().getExpr() = a.getRightOperand() and
222-
a.getLeftOperand().getAVariable().getName() = name
225+
AssignSensitiveVariableSource() {
226+
exists(SensitiveDataClassification classification |
227+
// avoid safe values assigned to presumably unsafe names
228+
not this instanceof NonCleartextSensitive and
229+
nameIndicatesSensitiveData(name, classification) and
230+
not classification = SensitiveDataClassification::id() and
231+
not nameIsNotSensitive(name) and
232+
exists(Assignment a |
233+
this.asExpr().getExpr() = a.getRightOperand() and
234+
a.getLeftOperand().getAVariable().getName() = name
235+
)
223236
)
224237
}
225238

226239
override string describe() { result = "an assignment to " + name }
227240
}
228241

229-
/** A parameter that may contain a password. */
230-
private class ParameterPasswordSource extends Source {
242+
/** A parameter that may contain sensitive data. */
243+
private class ParameterSensitiveSource extends Source {
231244
private string name;
232245

233-
ParameterPasswordSource() {
234-
name.regexpMatch(maybePassword()) and
235-
not nameIsNotSensitive(name) and
236-
not this instanceof NonCleartextPassword and
237-
exists(Parameter p, LocalVariable v |
238-
v = p.getAVariable() and
239-
v.getName() = name and
240-
this.asExpr().getExpr() = v.getAnAccess()
246+
ParameterSensitiveSource() {
247+
exists(SensitiveDataClassification classification |
248+
nameIndicatesSensitiveData(name, classification) and
249+
not classification = SensitiveDataClassification::id() and
250+
not nameIsNotSensitive(name) and
251+
not this instanceof NonCleartextSensitive and
252+
exists(Parameter p, LocalVariable v |
253+
v = p.getAVariable() and
254+
v.getName() = name and
255+
this.asExpr().getExpr() = v.getAnAccess()
256+
)
241257
)
242258
}
243259

@@ -260,10 +276,10 @@ module CleartextSources {
260276
deprecated predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
261277
exists(string name, ElementReference ref, LocalVariable hashVar |
262278
// from `hsh[password] = "changeme"` to a `hsh[password]` read
263-
nodeFrom.(HashKeyWritePasswordSource).getName() = name and
279+
nodeFrom.(HashKeyWriteSensitiveSource).getName() = name and
264280
nodeTo.asExpr().getExpr() = ref and
265281
ref.getArgument(0).getConstantValue().getStringlikeValue() = name and
266-
nodeFrom.(HashKeyWritePasswordSource).getVariable() = hashVar and
282+
nodeFrom.(HashKeyWriteSensitiveSource).getVariable() = hashVar and
267283
ref.getReceiver().(VariableReadAccess).getVariable() = hashVar and
268284
nodeFrom.asExpr().getASuccessor*() = nodeTo.asExpr()
269285
)

0 commit comments

Comments
 (0)