Skip to content

Commit b9a1a1f

Browse files
committed
JS: Rewrite to use nameIndicatesSensitiveData
I added this predicate mostly because it was nice with an easy shortcut for it, but also since I spotted the `CredentialsFunctionName` not checking agaisnt the regexps in `notSensitive`, which looked suspicious. So the main goal of adding `nameIndicatesSensitiveData` is that you don't accidentially forget to ensure that the name doesn't match against `notSensitve`.
1 parent b6f8e50 commit b9a1a1f

File tree

1 file changed

+8
-10
lines changed

1 file changed

+8
-10
lines changed

javascript/ql/src/semmle/javascript/security/SensitiveActions.qll

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ class SensitiveCall extends SensitiveExpr, InvokeExpr {
5050
// This is particularly to pick up methods with an argument like "password", which
5151
// may indicate a lookup.
5252
exists(string s | this.getAnArgument().mayHaveStringValue(s) |
53-
s.regexpMatch(maybeSensitive(classification)) and
54-
not s.regexpMatch(notSensitive())
53+
nameIndicatesSensitiveData(s, classification)
5554
)
5655
}
5756

@@ -84,10 +83,7 @@ private class BasicSensitiveWrite extends SensitiveWrite {
8483
SensitiveDataClassification classification;
8584

8685
BasicSensitiveWrite() {
87-
exists(string name |
88-
name.regexpMatch(maybeSensitive(classification)) and
89-
not name.regexpMatch(notSensitive())
90-
|
86+
exists(string name | nameIndicatesSensitiveData(name, classification) |
9187
exists(DataFlow::PropWrite pwn |
9288
pwn.getPropertyName() = name and
9389
pwn.getRhs() = this
@@ -109,9 +105,7 @@ private class BasicSensitiveWrite extends SensitiveWrite {
109105
private class BasicSensitiveVariableAccess extends SensitiveVariableAccess {
110106
SensitiveDataClassification classification;
111107

112-
BasicSensitiveVariableAccess() {
113-
name.regexpMatch(maybeSensitive(classification)) and not name.regexpMatch(notSensitive())
114-
}
108+
BasicSensitiveVariableAccess() { nameIndicatesSensitiveData(name, classification) }
115109

116110
override SensitiveDataClassification getClassification() { result = classification }
117111
}
@@ -135,7 +129,11 @@ abstract class SensitiveDataFunctionName extends SensitiveFunctionName {
135129
class CredentialsFunctionName extends SensitiveDataFunctionName {
136130
SensitiveDataClassification classification;
137131

138-
CredentialsFunctionName() { this.regexpMatch(maybeSensitive(classification)) }
132+
CredentialsFunctionName() {
133+
// TODO: is it by purpose that we don't check whether `this` does not
134+
// match the regexps in `notSensitive`?
135+
this.regexpMatch(maybeSensitive(classification))
136+
}
139137

140138
override SensitiveDataClassification getClassification() { result = classification }
141139
}

0 commit comments

Comments
 (0)