Skip to content

Commit 8ff515a

Browse files
committed
address review feedback on MaskingReplacer
1 parent 4ec2070 commit 8ff515a

3 files changed

Lines changed: 36 additions & 16 deletions

File tree

javascript/ql/src/semmle/javascript/security/dataflow/CleartextLoggingCustomizations.qll

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,19 @@ module CleartextLogging {
3434
abstract class Barrier extends DataFlow::Node { }
3535

3636
/**
37-
* A call to `.replace()` that seems to mask
37+
* A call to `.replace()` that seems to mask sensitive information.
3838
*/
3939
class MaskingReplacer extends Barrier, DataFlow::MethodCallNode {
40-
MaskingReplacer() {
41-
this.getCalleeName() = "replace" and
42-
exists(RegExpLiteral reg|
43-
reg = this.getArgument(0).getALocalSource().asExpr() and
44-
reg.getFlags().regexpMatch("(?i).*g.*") and
45-
reg.getRoot().getRawValue().regexpMatch(".*\\..*")
46-
)
47-
and
48-
this.getArgument(1).asExpr() instanceof StringLiteral
49-
}
40+
MaskingReplacer() {
41+
this.getCalleeName() = "replace" and
42+
exists(RegExpLiteral reg |
43+
reg = this.getArgument(0).getALocalSource().asExpr() and
44+
reg.isGlobal() and
45+
any(RegExpDot term).getLiteral() = reg
46+
)
47+
and
48+
exists(this.getArgument(1).getStringValue())
49+
}
5050
}
5151

5252
/**

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,14 @@ nodes
120120
| passwords.js:156:17:156:27 | process.env |
121121
| passwords.js:156:17:156:27 | process.env |
122122
| passwords.js:156:17:156:27 | process.env |
123-
| passwords.js:158:17:158:27 | process.env |
124-
| passwords.js:158:17:158:27 | process.env |
125-
| passwords.js:158:17:158:42 | process ... "bar"] |
126-
| passwords.js:158:17:158:42 | process ... "bar"] |
123+
| passwords.js:163:14:163:21 | password |
124+
| passwords.js:163:14:163:21 | password |
125+
| passwords.js:163:14:163:41 | passwor ... g, "*") |
126+
| passwords.js:163:14:163:41 | passwor ... g, "*") |
127+
| passwords.js:164:14:164:21 | password |
128+
| passwords.js:164:14:164:21 | password |
129+
| passwords.js:164:14:164:42 | passwor ... g, "*") |
130+
| passwords.js:164:14:164:42 | passwor ... g, "*") |
127131
| passwords_in_browser1.js:2:13:2:20 | password |
128132
| passwords_in_browser1.js:2:13:2:20 | password |
129133
| passwords_in_browser1.js:2:13:2:20 | password |
@@ -260,6 +264,14 @@ edges
260264
| passwords.js:154:21:154:28 | procdesc | passwords.js:142:26:142:34 | arguments |
261265
| passwords.js:154:21:154:28 | procdesc | passwords.js:142:26:142:34 | arguments |
262266
| passwords.js:156:17:156:27 | process.env | passwords.js:156:17:156:27 | process.env |
267+
| passwords.js:163:14:163:21 | password | passwords.js:163:14:163:41 | passwor ... g, "*") |
268+
| passwords.js:163:14:163:21 | password | passwords.js:163:14:163:41 | passwor ... g, "*") |
269+
| passwords.js:163:14:163:21 | password | passwords.js:163:14:163:41 | passwor ... g, "*") |
270+
| passwords.js:163:14:163:21 | password | passwords.js:163:14:163:41 | passwor ... g, "*") |
271+
| passwords.js:164:14:164:21 | password | passwords.js:164:14:164:42 | passwor ... g, "*") |
272+
| passwords.js:164:14:164:21 | password | passwords.js:164:14:164:42 | passwor ... g, "*") |
273+
| passwords.js:164:14:164:21 | password | passwords.js:164:14:164:42 | passwor ... g, "*") |
274+
| passwords.js:164:14:164:21 | password | passwords.js:164:14:164:42 | passwor ... g, "*") |
263275
| passwords_in_browser1.js:2:13:2:20 | password | passwords_in_browser1.js:2:13:2:20 | password |
264276
| passwords_in_browser2.js:2:13:2:20 | password | passwords_in_browser2.js:2:13:2:20 | password |
265277
| passwords_in_server_1.js:6:13:6:20 | password | passwords_in_server_1.js:6:13:6:20 | password |
@@ -301,6 +313,8 @@ edges
301313
| passwords.js:142:26:142:34 | arguments | passwords.js:150:21:150:31 | process.env | passwords.js:142:26:142:34 | arguments | Sensitive data returned by $@ is logged here. | passwords.js:150:21:150:31 | process.env | process environment |
302314
| passwords.js:142:26:142:34 | arguments | passwords.js:152:33:152:43 | process.env | passwords.js:142:26:142:34 | arguments | Sensitive data returned by $@ is logged here. | passwords.js:152:33:152:43 | process.env | process environment |
303315
| passwords.js:156:17:156:27 | process.env | passwords.js:156:17:156:27 | process.env | passwords.js:156:17:156:27 | process.env | Sensitive data returned by $@ is logged here. | passwords.js:156:17:156:27 | process.env | process environment |
316+
| passwords.js:163:14:163:41 | passwor ... g, "*") | passwords.js:163:14:163:21 | password | passwords.js:163:14:163:41 | passwor ... g, "*") | Sensitive data returned by $@ is logged here. | passwords.js:163:14:163:21 | password | an access to password |
317+
| passwords.js:164:14:164:42 | passwor ... g, "*") | passwords.js:164:14:164:21 | password | passwords.js:164:14:164:42 | passwor ... g, "*") | Sensitive data returned by $@ is logged here. | passwords.js:164:14:164:21 | password | an access to password |
304318
| passwords_in_server_1.js:6:13:6:20 | password | passwords_in_server_1.js:6:13:6:20 | password | 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 |
305319
| passwords_in_server_2.js:3:13:3:20 | password | passwords_in_server_2.js:3:13:3:20 | password | passwords_in_server_2.js:3:13:3:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_2.js:3:13:3:20 | password | an access to password |
306320
| passwords_in_server_3.js:2:13:2:20 | password | passwords_in_server_3.js:2:13:2:20 | password | passwords_in_server_3.js:2:13:2:20 | password | Sensitive data returned by $@ is logged here. | passwords_in_server_3.js:2:13:2:20 | password | an access to password |

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,10 @@ var Util = require('util');
156156
console.log(process.env); // NOT OK
157157
console.log(process.env.PATH); // OK.
158158
console.log(process.env["foo" + "bar"]); // OK.
159-
});
159+
});
160+
161+
(function () {
162+
console.log(password.replace(/./g, "*")); // OK!
163+
console.log(password.replace(/\./g, "*")); // NOT OK!
164+
console.log(password.replace(/foo/g, "*")); // NOT OK!
165+
})();

0 commit comments

Comments
 (0)