JavaScript: 1.18.1 cherry-picks

javascript/externs/web/w3c_dom1.js

@@ -25,9 +25,11 @@
 
 /**
  * @constructor
+ * @param {string=} message
+ * @param {string=} message
  * @see http://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-core.html#ID-17189187
  */
-function DOMException() {}
+function DOMException(message, name) {}
 
 /**
  * @type {number}

javascript/ql/src/Expressions/ImplicitOperandConversion.qhelp

@@ -39,7 +39,7 @@ property of the name stored in variable <code>member</code>:
 
 <p>
 However, this test is ineffective as written: the operator <code>!</code> binds more
-tighly than <code>in</code>, so it is applied first. Applying <code>!</code> to a
+tightly than <code>in</code>, so it is applied first. Applying <code>!</code> to a
 non-empty string yields <code>false</code>, so the <code>in</code> operator actually
 ends up checking whether <code>obj</code> contains a property called <code>"false"</code>.
 </p>

javascript/ql/src/LanguageFeatures/EmptyArrayInit.ql

@@ -45,5 +45,4 @@ class OmittedArrayElement extends ArrayExpr {
 }
 
 from OmittedArrayElement ae
-where not ae.getFile().getFileType().isTypeScript() // ignore quirks in TypeScript tokenizer
 select ae, "Avoid omitted array elements."

javascript/ql/src/LanguageFeatures/SemicolonInsertion.ql

@@ -36,8 +36,7 @@ where s.hasSemicolonInserted() and
       asi = strictcount(Stmt ss | asi(sc, ss, true)) and
       nstmt = strictcount(Stmt ss | asi(sc, ss, _)) and
       perc = ((1-asi/nstmt)*100).floor() and
-      perc >= 90 and
-      not s.getFile().getFileType().isTypeScript() // ignore some quirks in the TypeScript tokenizer
+      perc >= 90
 select (LastLineOf)s, "Avoid automated semicolon insertion " +
                       "(" + perc + "% of all statements in $@ have an explicit semicolon).",
                       sc, "the enclosing " + sctype

javascript/ql/src/Security/CWE-807/ConditionalBypass.ql

@@ -3,7 +3,7 @@
  * @description Conditions that the user controls are not suited for making security-related decisions.
  * @kind problem
  * @problem.severity error
- * @precision high
+ * @precision medium
  * @id js/user-controlled-bypass
  * @tags security
  *       external/cwe/cwe-807
@@ -83,8 +83,32 @@ predicate isTaintedGuardForSensitiveAction(Sink sink, DataFlow::Node source, Sen
   )
 }
 
+/**
+ * Holds if `e` effectively guards access to `action` by returning or throwing early.
+ *
+ * Example: `if (e) return; action(x)`.
+ */
+predicate isEarlyAbortGuard(Sink e, SensitiveAction action) {
+  exists (IfStmt guard |
+    // `e` is in the condition of an if-statement ...
+    e.asExpr().getParentExpr*() = guard.getCondition() and
+    // ... where the then-branch always throws or returns
+    exists (Stmt abort |
+      abort instanceof ThrowStmt or
+      abort instanceof ReturnStmt |
+      abort.nestedIn(guard) and
+      abort.getBasicBlock().(ReachableBasicBlock).postDominates(guard.getThen().getBasicBlock() )
+    ) and
+    // ... and the else-branch does not exist
+    not exists (guard.getElse()) |
+    // ... and `action` is outside the if-statement
+    not action.asExpr().getEnclosingStmt().nestedIn(guard)
+  )
+}
+
 from DataFlow::Node source, DataFlow::Node sink, SensitiveAction action
-where isTaintedGuardForSensitiveAction(sink, source, action)
+where isTaintedGuardForSensitiveAction(sink, source, action) and
+      not isEarlyAbortGuard(sink, action)
 select sink, "This condition guards a sensitive $@, but $@ controls it.",
     action, "action",
     source, "a user-provided value"

javascript/ql/src/Statements/MisleadingIndentationAfterControlStmt.ql

@@ -39,7 +39,6 @@ where misleadingIndentationCandidate(ctrl, s1, s2) and
       f.hasIndentation(ctrlStartLine, indent, _) and
       f.hasIndentation(startLine1, indent, _) and
       f.hasIndentation(startLine2, indent, _) and
-      not s2 instanceof EmptyStmt and
-      not f.getFileType().isTypeScript() // ignore quirks in TypeScript tokenizer
+      not s2 instanceof EmptyStmt
 select (FirstLineOf)s2, "The indentation of this statement suggests that it is controlled by $@, while in fact it is not.",
             (FirstLineOf)ctrl, "this statement"

javascript/ql/src/semmle/javascript/ES2015Modules.qll

@@ -27,7 +27,7 @@ class ES2015Module extends Module {
 
   /** Gets an export declaration in this module. */
   ExportDeclaration getAnExport() {
-    result.getContainer() = this
+    result.getTopLevel() = this
   }
 
   override Module getAnImportedModule() {
@@ -55,7 +55,7 @@ class ES2015Module extends Module {
 /** An import declaration. */
 class ImportDeclaration extends Stmt, Import, @importdeclaration {
   override ES2015Module getEnclosingModule() {
-    result = getContainer()
+    result = getTopLevel()
   }
 
   override PathExprInModule getImportedPath() {
@@ -254,7 +254,7 @@ class BulkReExportDeclaration extends ReExportDeclaration, @exportalldeclaration
   * but we ignore this subtlety.
   */
 private predicate isShadowedFromBulkExport(BulkReExportDeclaration reExport, string name) {
-  exists (ExportNamedDeclaration other | other.getContainer() = reExport.getEnclosingModule() |
+  exists (ExportNamedDeclaration other | other.getTopLevel() = reExport.getEnclosingModule() |
     other.getAnExportedDecl().getName() = name
     or
     other.getASpecifier().getExportedName() = name)

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

@@ -96,8 +96,7 @@ class AbstractProtoProperty extends AbstractProperty {
  * which in turn introduces a materialization.
  */
 private AbstractValue getAnAssignedValue(AbstractValue b, string p) {
-  exists (AnalyzedPropertyWrite apw, DataFlow::AnalyzedNode afn |
-    apw.writes(b, p, afn) and
-    result = afn.getALocalValue()
+  exists (AnalyzedPropertyWrite apw |
+    apw.writesValue(b, p, result)
   )
 }

javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll

@@ -268,6 +268,12 @@ private class AnalyzedVariableExport extends AnalyzedPropertyWrite, DataFlow::Va
     propName = name and
     source = varDef.getSource().analyze()
   }
+
+  override predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
+    baseVal = TAbstractExportsObject(export.getEnclosingModule()) and
+    propName = name and
+    val = varDef.getAnAssignedValue()
+  }
 }
 
 /**
@@ -301,7 +307,7 @@ private class AnalyzedExportAssign extends AnalyzedPropertyWrite, DataFlow::Valu
   }
 
   override predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
-    baseVal = TAbstractModuleObject(exportAssign.getContainer()) and
+    baseVal = TAbstractModuleObject(exportAssign.getTopLevel()) and
     propName = "exports" and
     source = this
   }

javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll

@@ -160,18 +160,26 @@ private class IIFEWithAnalyzedReturnFlow extends CallWithAnalyzedReturnFlow {
 
 }
 
+/**
+ * Gets the only access to `v`, which is the variable declared by `fn`.
+ *
+ * This predicate is not defined for global functions `fn`, or for
+ * local variables `v` that do not have exactly one access.
+ */
+private VarAccess getOnlyAccess(FunctionDeclStmt fn, LocalVariable v) {
+  v = fn.getVariable() and
+  result = v.getAnAccess() and
+  strictcount(v.getAnAccess()) = 1
+}
+
 /** A function that only is used locally, making it amenable to type inference. */
 class LocalFunction extends Function {
 
   DataFlow::Impl::ExplicitInvokeNode invk;
 
   LocalFunction() {
-    this instanceof FunctionDeclStmt and
-    exists (LocalVariable v, Expr callee |
-      callee = invk.getCalleeNode().asExpr() and
-      v = getVariable() and
-      v.getAnAccess() = callee and
-      forall(VarAccess o | o = v.getAnAccess() | o = callee) and
+    exists (LocalVariable v |
+      getOnlyAccess(this, v) = invk.getCalleeNode().asExpr() and
       not exists(v.getAnAssignedExpr()) and
       not exists(ExportDeclaration export | export.exportsAs(v, _))
     ) and

javascript/ql/src/semmle/javascript/dataflow/internal/PropertyTypeInference.qll

@@ -96,8 +96,24 @@ abstract class AnalyzedPropertyWrite extends DataFlow::Node {
   /**
    * Holds if this property write assigns `source` to property `propName` of one of the
    * concrete objects represented by `baseVal`.
+   *
+   * Note that not all property writes have an explicit `source` node; use predicate
+   * `writesValue` below to cover these cases.
    */
-  abstract predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source);
+  predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
+    none()
+  }
+
+  /**
+   * Holds if this property write assigns `val` to property `propName` of one of the
+   * concrete objects represented by `baseVal`.
+   */
+  predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
+    exists (AnalyzedNode source |
+      writes(baseVal, propName, source) and
+      val = source.getALocalValue()
+    )
+  }
 
   /**
    * Holds if the flow information for the base node of this property write is incomplete

javascript/ql/src/semmle/javascript/frameworks/SQL.qll

@@ -387,3 +387,105 @@ private module Sequelize {
     }
   }
 }
+
+/**
+ * Provides classes modelling the Google Cloud Spanner library.
+ */
+private module Spanner {
+  /**
+   * Gets a node that refers to the `Spanner` class
+   */
+  DataFlow::SourceNode spanner() {
+    // older versions
+    result = DataFlow::moduleImport("@google-cloud/spanner")
+    or
+    // newer versions
+    result = DataFlow::moduleMember("@google-cloud/spanner", "Spanner")
+  }
+
+  /**
+   * Gets a node that refers to an instance of the `Database` class.
+   */
+  DataFlow::SourceNode database() {
+    result = spanner().getAnInvocation().getAMethodCall("instance").getAMethodCall("database")
+  }
+
+  /**
+   * Gets a node that refers to an instance of the `v1.SpannerClient` class.
+   */
+  DataFlow::SourceNode v1SpannerClient() {
+    result = spanner().getAPropertyRead("v1").getAPropertyRead("SpannerClient").getAnInstantiation()
+  }
+
+  /**
+   * Gets a node that refers to a transaction object.
+   */
+  DataFlow::SourceNode transaction() {
+    result = database().getAMethodCall("runTransaction").getCallback(0).getParameter(1)
+  }
+
+  /**
+   * A call to a Spanner method that executes a SQL query.
+   */
+  abstract class SqlExecution extends DatabaseAccess, DataFlow::InvokeNode {
+    /**
+     * Gets the position of the query argument; default is zero, which can be overridden
+     * by subclasses.
+     */
+    int getQueryArgumentPosition() {
+      result = 0
+    }
+
+    override DataFlow::Node getAQueryArgument() {
+      result = getArgument(getQueryArgumentPosition()) or
+      result = getOptionArgument(getQueryArgumentPosition(), "sql")
+    }
+  }
+
+  /**
+   * A call to `Database.run`, `Database.runPartitionedUpdate` or `Database.runStream`.
+   */
+  class DatabaseRunCall extends SqlExecution {
+    DatabaseRunCall() {
+      exists (string run | run = "run" or run = "runPartitionedUpdate" or run = "runStream" |
+        this = database().getAMethodCall(run)
+      )
+    }
+  }
+
+  /**
+   * A call to `Transaction.run`, `Transaction.runStream` or `Transaction.runUpdate`.
+   */
+  class TransactionRunCall extends SqlExecution {
+    TransactionRunCall() {
+      exists (string run | run = "run" or run = "runStream" or run = "runUpdate" |
+        this = transaction().getAMethodCall(run)
+      )
+    }
+  }
+
+  /**
+   * A call to `v1.SpannerClient.executeSql` or `v1.SpannerClient.executeStreamingSql`.
+   */
+  class ExecuteSqlCall extends SqlExecution {
+    ExecuteSqlCall() {
+      exists (string exec | exec = "executeSql" or exec = "executeStreamingSql" |
+        this = v1SpannerClient().getAMethodCall(exec)
+      )
+    }
+
+    override DataFlow::Node getAQueryArgument() {
+      // `executeSql` and `executeStreamingSql` do not accept query strings directly
+      result = getOptionArgument(0, "sql")
+    }
+  }
+
+  /**
+   * An expression that is interpreted as a SQL string.
+   */
+  class QueryString extends SQL::SqlString {
+    QueryString() {
+      this = any(SqlExecution se).getAQueryArgument().asExpr()
+    }
+  }
+}

javascript/ql/test/library-tests/Flow/AbstractValues.expected

@@ -4,10 +4,12 @@
 | ChatListScreen.js:3:1:5:1 | instance of function foo |
 | a2.js:1:1:2:0 | exports object of module a2 |
 | a2.js:1:1:2:0 | module object of module a2 |
-| a.js:1:1:13:0 | exports object of module a |
-| a.js:1:1:13:0 | module object of module a |
+| a.js:1:1:18:0 | exports object of module a |
+| a.js:1:1:18:0 | module object of module a |
 | a.js:3:8:5:1 | function setX |
 | a.js:3:8:5:1 | instance of function setX |
+| a.js:15:1:17:1 | function bump |
+| a.js:15:1:17:1 | instance of function bump |
 | amd2.js:1:1:4:0 | exports object of module amd2 |
 | amd2.js:1:1:4:0 | module object of module amd2 |
 | amd2.js:1:8:3:1 | anonymous function |
@@ -36,8 +38,8 @@
 | arguments.js:30:2:33:1 | anonymous function |
 | arguments.js:30:2:33:1 | arguments object of anonymous function |
 | arguments.js:30:2:33:1 | instance of anonymous function |
-| b.js:1:1:55:0 | exports object of module b |
-| b.js:1:1:55:0 | module object of module b |
+| b.js:1:1:58:0 | exports object of module b |
+| b.js:1:1:58:0 | module object of module b |
 | backend.js:1:1:3:0 | exports object of module backend |
 | backend.js:1:1:3:0 | module object of module backend |
 | backend.js:1:17:1:18 | object literal |
@@ -180,6 +182,10 @@
 | n.js:3:16:3:23 | object literal |
 | namespace-reexport.js:1:1:4:0 | exports object of module namespace-reexport |
 | namespace-reexport.js:1:1:4:0 | module object of module namespace-reexport |
+| nestedImport.js:1:1:13:0 | exports object of module nestedImport |
+| nestedImport.js:1:1:13:0 | module object of module nestedImport |
+| nestedImport.js:9:1:12:1 | function tst |
+| nestedImport.js:9:1:12:1 | instance of function tst |
 | nodeJsClient.js:1:1:6:0 | exports object of module nodeJsClient |
 | nodeJsClient.js:1:1:6:0 | module object of module nodeJsClient |
 | nodeJsLib.js:1:1:4:0 | exports object of module nodeJsLib |

javascript/ql/test/library-tests/Flow/a.js

@@ -10,3 +10,8 @@ let z = someGlobal;
 
 export let w;
 w = "w";
+
+export let notAlwaysZero = 0;
+function bump() {
+  ++notAlwaysZero;
+}

javascript/ql/test/library-tests/Flow/abseval.expected

@@ -6,6 +6,7 @@
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | indefinite value (global) |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | non-zero value |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | true |
+| a.js:14:12:14:24 | notAlwaysZero | a.js:14:28:14:28 | 0 | file://:0:0:0:0 | 0 |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | amd.js:1:1:7:0 | module object of module amd |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | file://:0:0:0:0 | indefinite value (call) |
 | amd.js:3:7:3:7 | e | amd.js:3:11:3:13 | exp | amd.js:1:1:7:0 | exports object of module amd |
@@ -54,9 +55,12 @@
 | b.js:42:5:42:7 | z11 | b.js:42:11:42:18 | toString | file://:0:0:0:0 | indefinite value (import) |
 | b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:1:1:6:0 | exports object of module ts2 |
 | b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:1:10:1:22 | anonymous function |
+| b.js:45:5:45:7 | z12 | b.js:45:11:45:12 | f2 | ts2.ts:4:12:4:13 | object literal |
 | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | file://:0:0:0:0 | non-empty, non-numeric string |
 | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | file://:0:0:0:0 | indefinite value (import) |
 | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | file://:0:0:0:0 | indefinite value (import) |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | 0 |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | non-zero value |
 | backend.js:1:7:1:13 | Backend | backend.js:1:17:1:18 | {} | backend.js:1:17:1:18 | object literal |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (call) |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (heap) |
@@ -171,6 +175,10 @@
 | mixed.js:9:5:9:6 | fn | mixed.js:9:10:9:19 | __filename | file://:0:0:0:0 | numeric string |
 | mixed.js:10:5:10:6 | dn | mixed.js:10:10:10:18 | __dirname | file://:0:0:0:0 | non-empty, non-numeric string |
 | mixed.js:10:5:10:6 | dn | mixed.js:10:10:10:18 | __dirname | file://:0:0:0:0 | numeric string |
+| nestedImport.js:2:5:2:6 | x1 | nestedImport.js:2:10:2:12 | foo | esLib.js:3:8:3:24 | function foo |
+| nestedImport.js:6:7:6:8 | x2 | nestedImport.js:6:12:6:14 | foo | file://:0:0:0:0 | indefinite value (import) |
+| nestedImport.js:6:7:6:8 | x2 | nestedImport.js:6:12:6:14 | foo | nodeJsLib.js:3:15:3:37 | function nodeJsFoo |
+| nestedImport.js:11:7:11:8 | x3 | nestedImport.js:11:12:11:14 | foo | esLib.js:3:8:3:24 | function foo |
 | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | file://:0:0:0:0 | indefinite value (call) |
 | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | nodeJsLib.js:1:1:4:0 | exports object of module nodeJsLib |
 | nodeJsClient.js:1:5:1:6 | nj | nodeJsClient.js:1:10:1:31 | require ... JsLib') | nodeJsLib.js:1:18:1:43 | function nodeJsModule |