Skip to content

Commit 944a2ca

Browse files
committed
JS: Replace ClearTextLogging::isSanitizerEdge with a node
1 parent 68584e5 commit 944a2ca

File tree

4 files changed

+22
-9
lines changed

4 files changed

+22
-9
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/BuildArtifactLeakQuery.qll

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,6 @@ class Configuration extends TaintTracking::Configuration {
2727

2828
override predicate isSanitizer(DataFlow::Node node) { node instanceof CleartextLogging::Barrier }
2929

30-
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
31-
CleartextLogging::isSanitizerEdge(pred, succ)
32-
}
33-
3430
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
3531
CleartextLogging::isAdditionalTaintStep(src, trg)
3632
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,12 +175,24 @@ module CleartextLogging {
175175
}
176176

177177
/**
178+
* DEPRECATED. Use `Barrier` instead, sanitized have been replaced by sanitized nodes.
179+
*
178180
* Holds if the edge `pred` -> `succ` should be sanitized for clear-text logging of sensitive information.
179181
*/
180-
predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
182+
deprecated predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
181183
succ.(DataFlow::PropRead).getBase() = pred
182184
}
183185

186+
private class PropReadAsBarrier extends Barrier {
187+
PropReadAsBarrier() {
188+
this = any(DataFlow::PropRead read).getBase() and
189+
// the 'foo' in 'foo.bar()' may have flow, we only want to suppress plain property reads
190+
not this = any(DataFlow::MethodCallNode call).getReceiver() and
191+
// do not block custom taint steps from this node
192+
not isAdditionalTaintStep(this, _)
193+
}
194+
}
195+
184196
/**
185197
* Holds if the edge `src` -> `trg` is an additional taint-step for clear-text logging of sensitive information.
186198
*/

javascript/ql/lib/semmle/javascript/security/dataflow/CleartextLoggingQuery.qll

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ class Configuration extends TaintTracking::Configuration {
3333

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

36-
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
37-
CleartextLogging::isSanitizerEdge(pred, succ)
38-
}
39-
4036
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node trg) {
4137
CleartextLogging::isAdditionalTaintStep(src, trg)
4238
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ nodes
99
| build-leaks.js:14:18:14:20 | env |
1010
| build-leaks.js:15:24:15:34 | process.env |
1111
| build-leaks.js:15:24:15:34 | process.env |
12+
| build-leaks.js:15:24:15:39 | process.env[key] |
1213
| build-leaks.js:16:20:16:22 | env |
1314
| build-leaks.js:21:11:26:5 | stringifed |
1415
| build-leaks.js:21:24:26:5 | {\\n ... )\\n } |
1516
| build-leaks.js:22:24:25:14 | Object. ... }, {}) |
1617
| build-leaks.js:22:49:22:51 | env |
18+
| build-leaks.js:23:24:23:47 | JSON.st ... w[key]) |
1719
| build-leaks.js:23:39:23:41 | raw |
20+
| build-leaks.js:23:39:23:46 | raw[key] |
1821
| build-leaks.js:24:20:24:22 | env |
1922
| build-leaks.js:30:22:30:31 | stringifed |
2023
| build-leaks.js:34:26:34:57 | getEnv( ... ngified |
@@ -36,13 +39,19 @@ edges
3639
| build-leaks.js:14:18:14:20 | env | build-leaks.js:16:20:16:22 | env |
3740
| build-leaks.js:15:24:15:34 | process.env | build-leaks.js:14:18:14:20 | env |
3841
| build-leaks.js:15:24:15:34 | process.env | build-leaks.js:14:18:14:20 | env |
42+
| build-leaks.js:15:24:15:34 | process.env | build-leaks.js:15:24:15:39 | process.env[key] |
43+
| build-leaks.js:15:24:15:34 | process.env | build-leaks.js:15:24:15:39 | process.env[key] |
44+
| build-leaks.js:15:24:15:39 | process.env[key] | build-leaks.js:14:18:14:20 | env |
3945
| build-leaks.js:16:20:16:22 | env | build-leaks.js:13:17:19:10 | Object. ... }) |
4046
| build-leaks.js:16:20:16:22 | env | build-leaks.js:14:18:14:20 | env |
4147
| build-leaks.js:21:11:26:5 | stringifed | build-leaks.js:30:22:30:31 | stringifed |
4248
| build-leaks.js:21:24:26:5 | {\\n ... )\\n } | build-leaks.js:21:11:26:5 | stringifed |
4349
| build-leaks.js:22:24:25:14 | Object. ... }, {}) | build-leaks.js:21:24:26:5 | {\\n ... )\\n } |
4450
| build-leaks.js:22:49:22:51 | env | build-leaks.js:24:20:24:22 | env |
51+
| build-leaks.js:23:24:23:47 | JSON.st ... w[key]) | build-leaks.js:22:49:22:51 | env |
4552
| build-leaks.js:23:39:23:41 | raw | build-leaks.js:22:49:22:51 | env |
53+
| build-leaks.js:23:39:23:41 | raw | build-leaks.js:23:39:23:46 | raw[key] |
54+
| build-leaks.js:23:39:23:46 | raw[key] | build-leaks.js:23:24:23:47 | JSON.st ... w[key]) |
4655
| build-leaks.js:24:20:24:22 | env | build-leaks.js:22:24:25:14 | Object. ... }, {}) |
4756
| build-leaks.js:24:20:24:22 | env | build-leaks.js:22:49:22:51 | env |
4857
| build-leaks.js:30:22:30:31 | stringifed | build-leaks.js:34:26:34:57 | getEnv( ... ngified |

0 commit comments

Comments
 (0)