Skip to content

Commit 4ec2070

Browse files
erik-krogherik-krogh
committed
remove property reads on process.env as a taint step, and add a barrier for masking replace calls
1 parent 052a331 commit 4ec2070

4 files changed

Lines changed: 18 additions & 29 deletions

File tree

‎javascript/ql/src/semmle/javascript/security/dataflow/CleartextLogging.qll‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,10 @@ module CleartextLogging {
3434

3535
override predicate isSanitizer(DataFlow::Node node) { node instanceof Barrier }
3636

37-
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl) {
38-
// Only unknown property reads on sensitive objects propagate taint.
39-
(not lbl instanceof PartiallySensitiveMap or exists(succ.(DataFlow::PropRead).getPropertyName())) and
37+
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
4038
succ.(DataFlow::PropRead).getBase() = pred
4139
}
4240

43-
override predicate isAdditionalFlowStep(
44-
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
45-
) {
46-
trg.(DataFlow::PropRead).getBase() = src and
47-
inlbl instanceof PartiallySensitiveMap and
48-
outlbl.isData()
49-
}
50-
5141
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
5242
// A taint propagating data flow edge through objects: a tainted write taints the entire object.
5343
exists(DataFlow::PropWrite write |

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,22 @@ module CleartextLogging {
3232
* A barrier for clear-text logging of sensitive information.
3333
*/
3434
abstract class Barrier extends DataFlow::Node { }
35+
36+
/**
37+
* A call to `.replace()` that seems to mask
38+
*/
39+
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+
}
50+
}
3551

3652
/**
3753
* An argument to a logging mechanism.

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,27 +105,16 @@ nodes
105105
| passwords.js:137:17:137:24 | config.y |
106106
| passwords.js:137:17:137:24 | config.y |
107107
| passwords.js:142:26:142:34 | arguments |
108-
<<<<<<< HEAD
109-
| passwords.js:147:12:147:19 | password |
110-
| passwords.js:149:21:149:28 | config.x |
111-
| passwords.js:150:21:150:31 | process.env |
112-
=======
113108
| passwords.js:142:26:142:34 | arguments |
114109
| passwords.js:147:12:147:19 | password |
115110
| passwords.js:147:12:147:19 | password |
116111
| passwords.js:149:21:149:28 | config.x |
117112
| passwords.js:150:21:150:31 | process.env |
118113
| passwords.js:150:21:150:31 | process.env |
119-
>>>>>>> remove type cast, and fix expected test results
120114
| passwords.js:152:9:152:63 | procdesc |
121115
| passwords.js:152:20:152:44 | Util.in ... ss.env) |
122116
| passwords.js:152:20:152:63 | Util.in ... /g, '') |
123117
| passwords.js:152:33:152:43 | process.env |
124-
<<<<<<< HEAD
125-
| passwords.js:154:21:154:28 | procdesc |
126-
| passwords.js:156:17:156:27 | process.env |
127-
| passwords.js:158:17:158:27 | process.env |
128-
=======
129118
| passwords.js:152:33:152:43 | process.env |
130119
| passwords.js:154:21:154:28 | procdesc |
131120
| passwords.js:156:17:156:27 | process.env |
@@ -134,7 +123,6 @@ nodes
134123
| passwords.js:158:17:158:27 | process.env |
135124
| passwords.js:158:17:158:27 | process.env |
136125
| passwords.js:158:17:158:42 | process ... "bar"] |
137-
>>>>>>> remove type cast, and fix expected test results
138126
| passwords.js:158:17:158:42 | process ... "bar"] |
139127
| passwords_in_browser1.js:2:13:2:20 | password |
140128
| passwords_in_browser1.js:2:13:2:20 | password |
@@ -272,10 +260,6 @@ edges
272260
| passwords.js:154:21:154:28 | procdesc | passwords.js:142:26:142:34 | arguments |
273261
| passwords.js:154:21:154:28 | procdesc | passwords.js:142:26:142:34 | arguments |
274262
| passwords.js:156:17:156:27 | process.env | passwords.js:156:17:156:27 | process.env |
275-
| passwords.js:158:17:158:27 | process.env | passwords.js:158:17:158:42 | process ... "bar"] |
276-
| passwords.js:158:17:158:27 | process.env | passwords.js:158:17:158:42 | process ... "bar"] |
277-
| passwords.js:158:17:158:27 | process.env | passwords.js:158:17:158:42 | process ... "bar"] |
278-
| passwords.js:158:17:158:27 | process.env | passwords.js:158:17:158:42 | process ... "bar"] |
279263
| passwords_in_browser1.js:2:13:2:20 | password | passwords_in_browser1.js:2:13:2:20 | password |
280264
| passwords_in_browser2.js:2:13:2:20 | password | passwords_in_browser2.js:2:13:2:20 | password |
281265
| passwords_in_server_1.js:6:13:6:20 | password | passwords_in_server_1.js:6:13:6:20 | password |
@@ -317,7 +301,6 @@ edges
317301
| 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 |
318302
| 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 |
319303
| 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 |
320-
| passwords.js:158:17:158:42 | process ... "bar"] | passwords.js:158:17:158:27 | process.env | passwords.js:158:17:158:42 | process ... "bar"] | Sensitive data returned by $@ is logged here. | passwords.js:158:17:158:27 | process.env | process environment |
321304
| 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 |
322305
| 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 |
323306
| 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,5 +155,5 @@ var Util = require('util');
155155

156156
console.log(process.env); // NOT OK
157157
console.log(process.env.PATH); // OK.
158-
console.log(process.env["foo" + "bar"]); // NOT OK.
158+
console.log(process.env["foo" + "bar"]); // OK.
159159
});

0 commit comments

Comments
 (0)