Merge pull request #342 from xiemaisi/rc/1.18-cherry-picks

JavaScript: 1.18.1 cherry-picks

Author: Max Schaefer

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