Merge pull request #56 from xiemaisi/js/import-globals

Approved by asger-semmle

Author: semmle-qlci

change-notes/1.18/analysis-javascript.md

@@ -6,6 +6,8 @@
 
 * Modelling of data flow through destructuring assignments has been improved. This may give additional results for the security queries and other queries that rely on data flow.
 
+* Modelling of global variables has been improved. This may give more true-positive results and fewer false-positive results for a variety of queries.
+
 * Modelling of taint flow through the array operations `map` and `join` has been improved. This may give additional results for the security queries.
 
 * Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following libraries:
@@ -31,6 +33,7 @@
   - [extend2](https://github.com/eggjs/extend2)
   - [fast-json-parse](https://github.com/mcollina/fast-json-parse)
   - [forge](https://github.com/digitalbazaar/forge)
+  - [global](https://www.npmjs.com/package/global)
   - [he](https://github.com/mathiasbynens/he)
   - [html-entities](https://github.com/mdevils/node-html-entities)
   - [jquery](https://jquery.com)

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

@@ -219,17 +219,38 @@ class GlobalVarRefNode extends DataFlow::ValueNode, DataFlow::DefaultSourceNode
   string getName() { result = astNode.getName() }
 }
 
+/**
+ * Gets a data flow node corresponding to an access to the global object, including
+ * `this` expressions outside functions, references to global variables `window`
+ * and `global`, and uses of the `global` npm package.
+ */
+DataFlow::SourceNode globalObjectRef() {
+  // top-level `this`
+  exists (ThisNode globalThis | result = globalThis |
+    not exists(globalThis.getBinder())
+  )
+  or
+  // DOM
+  result = globalVarRef("window") or
+  // Node.js
+  result = globalVarRef("global") or
+  // `require("global")`
+  result = moduleImport("global")
+}
+
 /**
  * Gets a data flow node corresponding to an access to global variable `name`,
- * either directly or through `window` or `global`.
+ * either directly, through `window` or `global`, or through the `global` npm package.
  */
 pragma[nomagic]
 DataFlow::SourceNode globalVarRef(string name) {
-  result.(GlobalVarRefNode).getName() = name or
-  // DOM environment
-  result = globalVarRef("window").getAPropertyReference(name) or
-  // Node.js environment
-  result = globalVarRef("global").getAPropertyReference(name)
+  result.(GlobalVarRefNode).getName() = name
+  or
+  result = globalObjectRef().getAPropertyReference(name)
+  or
+  // `require("global/document")` or `require("global/window")`
+  (name = "document" or name = "window") and
+  result = moduleImport("global/" + name)
 }
 
 /** A data flow node corresponding to a function definition. */

javascript/ql/test/library-tests/Nodes/globalObjectRef.expected

@@ -0,0 +1,10 @@
+| tst2.js:1:9:1:25 | require("global") |
+| tst2.js:3:1:3:24 | require ... indow") |
+| tst2.js:7:1:7:6 | global |
+| tst2.js:8:1:8:6 | global |
+| tst2.js:9:1:9:4 | this |
+| tst.js:1:1:1:6 | window |
+| tst.js:2:1:2:4 | this |
+| tst.js:3:1:3:6 | window |
+| tst.js:4:1:4:6 | window |
+| tst.js:4:1:4:13 | window.window |

javascript/ql/test/library-tests/Nodes/globalObjectRef.ql

@@ -0,0 +1,3 @@
+import javascript
+
+select DataFlow::globalObjectRef()

javascript/ql/test/library-tests/Nodes/globalVarRef.expected

@@ -0,0 +1,13 @@
+| Object | tst2.js:8:1:8:13 | global.Object |
+| String | tst2.js:9:1:9:11 | this.String |
+| document | tst2.js:2:1:2:26 | require ... ument") |
+| document | tst.js:3:1:3:15 | window.document |
+| document | tst.js:4:1:4:22 | window. ... ocument |
+| global | tst2.js:7:1:7:6 | global |
+| global | tst2.js:8:1:8:6 | global |
+| setTimeout | tst2.js:5:1:5:12 | g.setTimeout |
+| window | tst2.js:3:1:3:24 | require ... indow") |
+| window | tst.js:1:1:1:6 | window |
+| window | tst.js:3:1:3:6 | window |
+| window | tst.js:4:1:4:6 | window |
+| window | tst.js:4:1:4:13 | window.window |

javascript/ql/test/library-tests/Nodes/globalVarRef.ql

@@ -0,0 +1,4 @@
+import javascript
+
+from string n
+select n, DataFlow::globalVarRef(n)

javascript/ql/test/library-tests/Nodes/tst.js

@@ -0,0 +1,4 @@
+window;
+this;
+window.document;
+window.window.document;

javascript/ql/test/library-tests/Nodes/tst2.js

@@ -0,0 +1,9 @@
+var g = require("global");
+require("global/document");
+require("global/window");
+
+g.setTimeout;
+
+global;
+global.Object;
+this.String;

javascript/ql/test/query-tests/DOM/PseudoEval/PseudoEval.expected

@@ -2,3 +2,4 @@
 | tst.js:2:1:2:24 | setInte ... te();") | Avoid using functions that evaluate strings as code. |
 | tst.js:4:1:4:31 | documen ... i!');") | Avoid using functions that evaluate strings as code. |
 | tst.js:5:1:5:30 | window. ... gger;") | Avoid using functions that evaluate strings as code. |
+| tst.js:9:3:9:33 | documen ... d = 2") | Avoid using functions that evaluate strings as code. |

javascript/ql/test/query-tests/DOM/PseudoEval/tst.js

@@ -2,4 +2,9 @@ window.setTimeout(";");
 setInterval("update();");
 setInterval(update);
 document.write("alert('Hi!');");
-window.execScript("debugger;");
+window.execScript("debugger;");
+
+(function(global) {
+  var document = global.document;
+  document.write("undefined = 2");
+})(this);