Merge pull request #4686 from erik-krogh/buildFp

Approved by esbena

Author: codeql-ci

javascript/change-notes/2020-12-16-build-artifact-leak.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The `js/build-artifact-leak` query no longer reports when only a safe subset of the properties on `process.env` are included in a build-artifact.

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

@@ -4,8 +4,6 @@
  */
 
 import javascript
-private import semmle.javascript.dataflow.InferredTypes
-private import semmle.javascript.security.SensitiveActions::HeuristicNames
 
 /**
  * Sinks for storage of sensitive information in build artifact.

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

@@ -204,6 +204,7 @@ module CleartextLogging {
     |
       not exists(write.getPropertyName()) and
       not exists(read.getPropertyName()) and
+      not isFilteredPropertyName(read.getPropertyNameExpr().flow().getALocalSource()) and
       src = read.getBase() and
       trg = write.getBase().getALocalSource()
     )
@@ -216,4 +217,24 @@ module CleartextLogging {
       trg.asExpr() = f.getArgumentsVariable().getAnAccess()
     )
   }
+
+  /**
+   * Holds if `name` is filtered by e.g. a regular-expression test or a filter call.
+   */
+  private predicate isFilteredPropertyName(DataFlow::Node name) {
+    exists(DataFlow::MethodCallNode reduceCall |
+      reduceCall.getABoundCallbackParameter(0, 1).flowsTo(name) and
+      reduceCall.getMethodName() = "reduce"
+    |
+      reduceCall.getReceiver+().(DataFlow::MethodCallNode).getMethodName() = "filter"
+    )
+    or
+    exists(StringOps::RegExpTest test |
+      test.getStringOperand().getALocalSource() = name.getALocalSource()
+    )
+    or
+    exists(MembershipCandidate test |
+      test.getAMemberNode().getALocalSource() = name.getALocalSource()
+    )
+  }
 }

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

@@ -40,3 +40,54 @@ var server = https.createServer(function (req, res) {
     let pw = url.parse(req.url, true).query.current_password;
     var plugin = new webpack.DefinePlugin({ "process.env.secret": JSON.stringify(pw) }); // NOT OK
 });
+
+(function () {
+    const REACT_APP = /^REACT_APP_/i;
+
+    function getOnlyReactVariables() {
+        const raw = Object.keys(process.env)
+            .filter(key => REACT_APP.test(key)) // This filters makes it safe.
+            .reduce(
+                (env, key) => {
+                    env[key] = process.env[key];
+                    return env;
+                },
+                {}
+            );
+        return raw;
+    }
+
+    new webpack.DefinePlugin(getOnlyReactVariables()); // OK
+
+    function getOnlyReactVariables2() {
+        const raw = Object.keys(process.env)
+            .reduce(
+                (env, key) => {
+                    if (REACT_APP.test(key)) {
+                        env[key] = process.env[key];
+                    }
+                    return env;
+                },
+                {}
+            );
+        return raw;
+    }
+
+    new webpack.DefinePlugin(getOnlyReactVariables2()); // OK
+
+    function getOnlyReactVariables3() {
+        const raw = Object.keys(process.env)
+            .reduce(
+                (env, key) => {
+                    if (key == ["1", "2", "3"]) {
+                        env[key] = process.env[key];
+                    }
+                    return env;
+                },
+                {}
+            );
+        return raw;
+    }
+
+    new webpack.DefinePlugin(getOnlyReactVariables3()); // OK
+})();