Merge pull request github#3555 from max-schaefer/js/require-flow

Approved by asgerf

Author: semmle-qlci

javascript/ql/src/semmle/javascript/NodeJS.qll

@@ -152,6 +152,18 @@ private class RequireVariable extends Variable {
  */
 private predicate moduleInFile(Module m, File f) { m.getFile() = f }
 
+/**
+ * Holds if `nd` may refer to `require`, either directly or modulo local data flow.
+ */
+cached
+private predicate isRequire(DataFlow::Node nd) {
+  nd.asExpr() = any(RequireVariable req).getAnAccess() and
+  // `mjs` files explicitly disallow `require`
+  not nd.getFile().getExtension() = "mjs"
+  or
+  isRequire(nd.getAPredecessor())
+}
+
 /**
  * A `require` import.
  *
@@ -162,12 +174,7 @@ private predicate moduleInFile(Module m, File f) { m.getFile() = f }
  * ```
  */
 class Require extends CallExpr, Import {
-  cached
-  Require() {
-    any(RequireVariable req).getAnAccess() = getCallee() and
-    // `mjs` files explicitly disallow `require`
-    not getFile().getExtension() = "mjs"
-  }
+  Require() { isRequire(getCallee().flow()) }
 
   override PathExpr getImportedPath() { result = getArgument(0) }
 
@@ -257,8 +264,8 @@ private class RequirePath extends PathExprCandidate {
   RequirePath() {
     this = any(Require req).getArgument(0)
     or
-    exists(RequireVariable req, MethodCallExpr reqres |
-      reqres.getReceiver() = req.getAnAccess() and
+    exists(MethodCallExpr reqres |
+      isRequire(reqres.getReceiver().flow()) and
       reqres.getMethodName() = "resolve" and
       this = reqres.getArgument(0)
     )

javascript/ql/test/library-tests/NodeJS/Require.expected

@@ -11,6 +11,8 @@
 | d.js:7:1:7:14 | require('foo') |
 | e.js:5:1:5:18 | require("process") |
 | f.js:2:1:2:7 | r("fs") |
+| g.js:1:1:1:96 | (proces ... https") |
+| g.js:1:43:1:61 | require("electron") |
 | index.js:1:12:1:26 | require('path') |
 | index.js:2:1:2:41 | require ... b.js")) |
 | mjs-files/require-from-js.js:1:12:1:36 | require ... on-me') |

javascript/ql/test/library-tests/NodeJS/g.js

@@ -0,0 +1 @@
+(process && "renderer" === process.type ? require("electron").remote.require : require)("https");