Skip to content

Commit 2b9f5c3

Browse files
author
Esben Sparre Andreasen
committed
JS: remove check for test-environment in js/clear-text-logging
1 parent 3636708 commit 2b9f5c3

File tree

3 files changed

+11
-23
lines changed

3 files changed

+11
-23
lines changed

javascript/ql/src/Security/CWE-312/CleartextLogging.ql

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,8 @@ predicate inBrowserEnvironment(TopLevel tl) {
3131
)
3232
}
3333

34-
/**
35-
* Holds if `sink` only is reachable in a "test" environment.
36-
*/
37-
predicate inTestEnvironment(Sink sink) {
38-
exists (IfStmt guard, Identifier id |
39-
// heuristic: a deliberate environment choice by the programmer related to passwords implies a test environment
40-
id.getName().regexpMatch("(?i).*(test|develop|production).*") and
41-
id.(Expr).getParentExpr*() = guard.getCondition() and
42-
(
43-
guard.getAControlledStmt() = sink.asExpr().getEnclosingStmt() or
44-
guard.getAControlledStmt().(BlockStmt).getAChildStmt() = sink.asExpr().getEnclosingStmt()
45-
)
46-
)
47-
}
48-
4934
from Configuration cfg, Source source, DataFlow::Node sink
5035
where cfg.hasFlow(source, sink) and
5136
// ignore logging to the browser console (even though it is not a good practice)
52-
not inBrowserEnvironment(sink.asExpr().getTopLevel()) and
53-
// ignore logging when testing
54-
not inTestEnvironment(sink)
37+
not inBrowserEnvironment(sink.asExpr().getTopLevel())
5538
select sink, "Sensitive data returned by $@ is logged here.", source, source.describe()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@
1111
| passwords.js:29:17:29:20 | obj3 | Sensitive data returned by $@ is logged here. | passwords.js:30:14:30:21 | password | an access to password |
1212
| passwords.js:78:17:78:38 | temp.en ... assword | Sensitive data returned by $@ is logged here. | passwords.js:77:37:77:53 | req.body.password | an access to password |
1313
| passwords.js:81:17:81:31 | `pw: ${secret}` | Sensitive data returned by $@ is logged here. | passwords.js:80:18:80:25 | password | an access to password |
14+
| passwords.js:93:21:93:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:93:39:93:46 | password | an access to password |
15+
| passwords.js:98:21:98:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:98:39:98:46 | password | an access to password |
16+
| passwords.js:105:21:105:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:105:39:105:46 | password | an access to password |
17+
| passwords.js:110:21:110:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:110:39:110:46 | password | an access to password |
1418
| passwords.js:114:25:114:50 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:114:43:114:50 | password | an access to password |
19+
| passwords.js:119:21:119:46 | "Passwo ... assword | Sensitive data returned by $@ is logged here. | passwords.js:119:39:119:46 | password | an access to password |
1520
| passwords.js:122:17:122:49 | name + ... tring() | Sensitive data returned by $@ is logged here. | passwords.js:122:31:122:38 | password | an access to password |
1621
| passwords.js:123:17:123:48 | name + ... lueOf() | Sensitive data returned by $@ is logged here. | passwords.js:123:31:123:38 | password | an access to password |
1722
| passwords_in_server_1.js:6:13:6:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_1.js:6:13:6:20 | password | an access to password |

javascript/ql/test/query-tests/Security/CWE-312/passwords.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,24 @@
9090
console.log("Password is: " + redact('password', password));
9191

9292
if (environment.isTestEnv()) {
93-
console.log("Password is: " + password); // OK
93+
console.log("Password is: " + password); // OK, but still flagged
9494
}
9595

9696
if (environment.is(TEST)) {
9797
// NB: for security reasons, we only log passwords in test environments
98-
console.log("Password is: " + password); // OK
98+
console.log("Password is: " + password); // OK, but still flagged
9999
}
100100

101101

102102
if (x.test(y)) {
103103
f();
104104
// ...
105-
console.log("Password is: " + password); // NOT OK, but not flagged
105+
console.log("Password is: " + password); // NOT OK
106106
// ...
107107
}
108108

109109
if (environment.isTestEnv())
110-
console.log("Password is: " + password); // OK
110+
console.log("Password is: " + password); // OK, but still flagged
111111

112112
if (x.test(y)) {
113113
if (f()) {
@@ -116,7 +116,7 @@
116116
}
117117

118118
if (!environment.isProduction()) {
119-
console.log("Password is: " + password); // OK
119+
console.log("Password is: " + password); // OK, but still flagged
120120
}
121121

122122
console.log(name + ", " + password.toString()); // NOT OK

0 commit comments

Comments
 (0)