JS: Fix regression from global declare vars

Author: asgerf

javascript/ql/lib/semmle/javascript/Variables.qll

@@ -126,6 +126,23 @@ class NamespaceScope extends Scope, @namespace_scope {
   override string toString() { result = "namespace scope" }
 }
 
+/**
+ * Holds if `v` is declared in the top-level of a module using a `declare` statement.
+ *
+ * For example:
+ * ```js
+ * declare var $: any;
+ * ```
+ *
+ * This introduces a `$` variable in the module scope, but it is likely just a way to provide
+ * a type for a global variable. We therefore treat it as a global variable.
+ */
+pragma[nomagic]
+private predicate isGlobalLike(Variable v) {
+  v.getScope() instanceof ModuleScope and
+  forex(VarDecl decl | decl = v.getADeclaration() | decl.isAmbient())
+}
+
 /** A variable declared in a scope. */
 class Variable extends @variable, LexicalName {
   /** Gets the name of this variable. */
@@ -135,7 +152,7 @@ class Variable extends @variable, LexicalName {
   override Scope getScope() { variables(this, _, result) }
 
   /** Holds if this is a global variable. */
-  predicate isGlobal() { this.getScope() instanceof GlobalScope }
+  predicate isGlobal() { this.getScope() instanceof GlobalScope or isGlobalLike(this) }
 
   /**
    * Holds if this is a variable exported from a TypeScript namespace.

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

@@ -62,6 +62,8 @@
 | dragAndDrop.ts:73:29:73:39 | droppedHtml | dragAndDrop.ts:71:27:71:61 | e.dataT ... /html') | dragAndDrop.ts:73:29:73:39 | droppedHtml | Cross-site scripting vulnerability due to $@. | dragAndDrop.ts:71:27:71:61 | e.dataT ... /html') | user-provided value |
 | event-handler-receiver.js:2:31:2:83 | '<h2><a ... ></h2>' | event-handler-receiver.js:2:49:2:61 | location.href | event-handler-receiver.js:2:31:2:83 | '<h2><a ... ></h2>' | Cross-site scripting vulnerability due to $@. | event-handler-receiver.js:2:49:2:61 | location.href | user-provided value |
 | express.js:6:15:6:33 | req.param("wobble") | express.js:6:15:6:33 | req.param("wobble") | express.js:6:15:6:33 | req.param("wobble") | Cross-site scripting vulnerability due to $@. | express.js:6:15:6:33 | req.param("wobble") | user-provided value |
+| jquery-declare-any.ts:6:7:6:17 | window.name | jquery-declare-any.ts:6:7:6:17 | window.name | jquery-declare-any.ts:6:7:6:17 | window.name | Cross-site scripting vulnerability due to $@. | jquery-declare-any.ts:6:7:6:17 | window.name | user-provided value |
+| jquery-declare-type.ts:6:7:6:17 | window.name | jquery-declare-type.ts:6:7:6:17 | window.name | jquery-declare-type.ts:6:7:6:17 | window.name | Cross-site scripting vulnerability due to $@. | jquery-declare-type.ts:6:7:6:17 | window.name | user-provided value |
 | jquery.js:7:5:7:34 | "<div i ... + "\\">" | jquery.js:2:17:2:40 | documen ... .search | jquery.js:7:5:7:34 | "<div i ... + "\\">" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
 | jquery.js:8:18:8:34 | "XSS: " + tainted | jquery.js:2:17:2:40 | documen ... .search | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:40 | documen ... .search | user-provided value |
 | jquery.js:10:5:10:40 | "<b>" + ...  "</b>" | jquery.js:10:13:10:20 | location | jquery.js:10:5:10:40 | "<b>" + ...  "</b>" | Cross-site scripting vulnerability due to $@. | jquery.js:10:13:10:20 | location | user-provided value |
@@ -954,6 +956,8 @@ nodes
 | event-handler-receiver.js:2:31:2:83 | '<h2><a ... ></h2>' | semmle.label | '<h2><a ... ></h2>' |
 | event-handler-receiver.js:2:49:2:61 | location.href | semmle.label | location.href |
 | express.js:6:15:6:33 | req.param("wobble") | semmle.label | req.param("wobble") |
+| jquery-declare-any.ts:6:7:6:17 | window.name | semmle.label | window.name |
+| jquery-declare-type.ts:6:7:6:17 | window.name | semmle.label | window.name |
 | jquery.js:2:7:2:40 | tainted | semmle.label | tainted |
 | jquery.js:2:17:2:40 | documen ... .search | semmle.label | documen ... .search |
 | jquery.js:4:5:4:11 | tainted | semmle.label | tainted |

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

@@ -182,6 +182,8 @@ nodes
 | hana.js:85:35:85:54 | tableRows[0].comment | semmle.label | tableRows[0].comment |
 | hana.js:90:33:90:34 | rs | semmle.label | rs |
 | hana.js:90:33:90:45 | rs[0].comment | semmle.label | rs[0].comment |
+| jquery-declare-any.ts:6:7:6:17 | window.name | semmle.label | window.name |
+| jquery-declare-type.ts:6:7:6:17 | window.name | semmle.label | window.name |
 | jquery.js:2:7:2:40 | tainted | semmle.label | tainted |
 | jquery.js:2:17:2:40 | documen ... .search | semmle.label | documen ... .search |
 | jquery.js:4:5:4:11 | tainted | semmle.label | tainted |

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery-declare-any.ts

@@ -3,5 +3,5 @@ import 'dummy';
 declare var $: any;
 
 function t() {
-    $(window.name); // $ MISSING: Alert
+    $(window.name); // $ Alert
 }

javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/jquery-declare-type.ts

@@ -3,5 +3,5 @@ import 'dummy';
 declare var $: JQueryStatic;
 
 function t() {
-    $(window.name); // $ MISSING: Alert
+    $(window.name); // $ Alert
 }