Skip to content

Commit 6482871

Browse files
committed
remove FPs in js/build-artifact-leak where the "leaked" properties are constrained to a safe subset
1 parent 06733ea commit 6482871

File tree

2 files changed

+72
-0
lines changed

2 files changed

+72
-0
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ module CleartextLogging {
205205
|
206206
not exists(write.getPropertyName()) and
207207
not exists(read.getPropertyName()) and
208+
not isFilteredPropertyName(read.getPropertyNameExpr().flow().getALocalSource()) and
208209
src = read.getBase() and
209210
trg = write.getBase().getALocalSource()
210211
)
@@ -217,4 +218,24 @@ module CleartextLogging {
217218
trg.asExpr() = f.getArgumentsVariable().getAnAccess()
218219
)
219220
}
221+
222+
/**
223+
* Holds if `name` is filtered by e.g. a regular-expression test or a filter call.
224+
*/
225+
private predicate isFilteredPropertyName(DataFlow::Node name) {
226+
exists(DataFlow::MethodCallNode reduceCall |
227+
reduceCall.getABoundCallbackParameter(0, 1).flowsTo(name) and
228+
reduceCall.getMethodName() = "reduce"
229+
|
230+
reduceCall.getReceiver+().(DataFlow::MethodCallNode).getMethodName() = "filter"
231+
)
232+
or
233+
exists(StringOps::RegExpTest test |
234+
test.getStringOperand().getALocalSource() = name.getALocalSource()
235+
)
236+
or
237+
exists(MembershipCandidate test |
238+
test.getAMemberNode().getALocalSource() = name.getALocalSource()
239+
)
240+
}
220241
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,54 @@ var server = https.createServer(function (req, res) {
4040
let pw = url.parse(req.url, true).query.current_password;
4141
var plugin = new webpack.DefinePlugin({ "process.env.secret": JSON.stringify(pw) }); // NOT OK
4242
});
43+
44+
(function () {
45+
const REACT_APP = /^REACT_APP_/i;
46+
47+
function getOnlyReactVariables() {
48+
const raw = Object.keys(process.env)
49+
.filter(key => REACT_APP.test(key)) // This filters makes it safe.
50+
.reduce(
51+
(env, key) => {
52+
env[key] = process.env[key];
53+
return env;
54+
},
55+
{}
56+
);
57+
return raw;
58+
}
59+
60+
new webpack.DefinePlugin(getOnlyReactVariables()); // OK
61+
62+
function getOnlyReactVariables2() {
63+
const raw = Object.keys(process.env)
64+
.reduce(
65+
(env, key) => {
66+
if (REACT_APP.test(key)) {
67+
env[key] = process.env[key];
68+
}
69+
return env;
70+
},
71+
{}
72+
);
73+
return raw;
74+
}
75+
76+
new webpack.DefinePlugin(getOnlyReactVariables2()); // OK
77+
78+
function getOnlyReactVariables3() {
79+
const raw = Object.keys(process.env)
80+
.reduce(
81+
(env, key) => {
82+
if (key == ["1", "2", "3"]) {
83+
env[key] = process.env[key];
84+
}
85+
return env;
86+
},
87+
{}
88+
);
89+
return raw;
90+
}
91+
92+
new webpack.DefinePlugin(getOnlyReactVariables3()); // OK
93+
})();

0 commit comments

Comments
 (0)