Skip to content

Commit 8d49f26

Browse files
authored
Merge pull request #20397 from asgerf/js/build-artifact-leak-fp
JS: Fix FP in js/build-artifact-leak when keys come from an array of constants
2 parents 227e1fc + 2a4d683 commit 8d49f26

File tree

4 files changed

+30
-5
lines changed

4 files changed

+30
-5
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,24 @@ module CleartextLogging {
247247
reduceCall.getABoundCallbackParameter(0, 1) = name
248248
|
249249
reduceCall.getReceiver+().(DataFlow::MethodCallNode).getMethodName() = "filter"
250+
or
251+
isArrayOfConstants(reduceCall.getReceiver+())
250252
)
251253
or
252254
exists(StringOps::RegExpTest test | test.getStringOperand().getALocalSource() = name)
253255
or
254256
exists(MembershipCandidate test | test.getAMemberNode().getALocalSource() = name)
255257
}
258+
259+
private predicate isArrayOfConstants(DataFlow::ArrayCreationNode array) {
260+
forex(DataFlow::Node node |
261+
node =
262+
[
263+
array.getAnElement(), array.getAPropertyWrite().getRhs(),
264+
array.getAMethodCall("push").getArgument(0)
265+
]
266+
|
267+
exists(node.getStringValue())
268+
)
269+
}
256270
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
#select
2+
| build-leaks.js:4:39:6:1 | {\\n " ... leak]\\n} | build-leaks.js:5:35:5:45 | process.env | build-leaks.js:4:39:6:1 | {\\n " ... leak]\\n} | This creates a build artifact that depends on $@. | build-leaks.js:5:35:5:45 | process.env | sensitive data returned byprocess environment |
3+
| build-leaks.js:34:26:34:57 | getEnv( ... ngified | build-leaks.js:15:24:15:34 | process.env | build-leaks.js:34:26:34:57 | getEnv( ... ngified | This creates a build artifact that depends on $@. | build-leaks.js:15:24:15:34 | process.env | sensitive data returned byprocess environment |
4+
| build-leaks.js:41:43:41:86 | { "proc ... y(pw) } | build-leaks.js:40:14:40:60 | url.par ... assword | build-leaks.js:41:43:41:86 | { "proc ... y(pw) } | This creates a build artifact that depends on $@. | build-leaks.js:40:14:40:60 | url.par ... assword | sensitive data returned byan access to current_password |
15
edges
26
| build-leaks.js:5:20:5:46 | JSON.st ... ss.env) | build-leaks.js:4:39:6:1 | {\\n " ... leak]\\n} | provenance | |
37
| build-leaks.js:5:35:5:45 | process.env | build-leaks.js:5:20:5:46 | JSON.st ... ss.env) | provenance | |
@@ -53,7 +57,3 @@ nodes
5357
subpaths
5458
| build-leaks.js:22:36:22:38 | raw | build-leaks.js:22:49:22:51 | env | build-leaks.js:24:20:24:22 | env | build-leaks.js:22:24:25:14 | Object. ... }, {}) |
5559
| build-leaks.js:22:36:22:38 | raw | build-leaks.js:23:39:23:41 | raw | build-leaks.js:24:20:24:22 | env | build-leaks.js:22:24:25:14 | Object. ... }, {}) |
56-
#select
57-
| build-leaks.js:4:39:6:1 | {\\n " ... leak]\\n} | build-leaks.js:5:35:5:45 | process.env | build-leaks.js:4:39:6:1 | {\\n " ... leak]\\n} | This creates a build artifact that depends on $@. | build-leaks.js:5:35:5:45 | process.env | sensitive data returned byprocess environment |
58-
| build-leaks.js:34:26:34:57 | getEnv( ... ngified | build-leaks.js:15:24:15:34 | process.env | build-leaks.js:34:26:34:57 | getEnv( ... ngified | This creates a build artifact that depends on $@. | build-leaks.js:15:24:15:34 | process.env | sensitive data returned byprocess environment |
59-
| build-leaks.js:41:43:41:86 | { "proc ... y(pw) } | build-leaks.js:40:14:40:60 | url.par ... assword | build-leaks.js:41:43:41:86 | { "proc ... y(pw) } | This creates a build artifact that depends on $@. | build-leaks.js:40:14:40:60 | url.par ... assword | sensitive data returned byan access to current_password |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
query: Security/CWE-312/BuildArtifactLeak.ql
2+
postprocess: utils/test/InlineExpectationsTestQuery.ql

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,14 @@ var server = https.createServer(function (req, res) {
9090
}
9191

9292
new webpack.DefinePlugin(getOnlyReactVariables3());
93-
})();
93+
94+
function getFilteredEnv4() {
95+
return ["FOO", "BAR", "BAZ"]
96+
.reduce((env, key) => {
97+
env[key] = JSON.stringify(process.env[key]);
98+
return env;
99+
}, {});
100+
}
101+
102+
new webpack.DefinePlugin(getFilteredEnv4());
103+
})();

0 commit comments

Comments
 (0)