Merge branch 'master' into js/call-graph-exploration

Author: erik-krogh

change-notes/1.25/analysis-javascript.md

@@ -20,3 +20,4 @@
 ## Changes to libraries
 
 * A library `semmle.javascript.explore.CallGraph` has been added to help write queries for exploring the call graph.
+* Added data flow for `Map` and `Set`, and added matching type-tracking steps that can accessed using the `CollectionsTypeTracking` module.

csharp/autobuilder/Semmle.Autobuild/CommandBuilder.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Linq;
 using System.Text;
 
@@ -169,7 +169,7 @@ void NextCommand()
                 arguments.Append(" &&");
         }
 
-        public CommandBuilder RunCommand(string exe, string? argumentsOpt = null)
+        public CommandBuilder RunCommand(string exe, string? argumentsOpt = null, bool quoteExe = true)
         {
             var (exe0, arg0) =
                 escapingMode == EscapeMode.Process && exe.EndsWith(".exe", System.StringComparison.Ordinal)
@@ -183,7 +183,10 @@ public CommandBuilder RunCommand(string exe, string? argumentsOpt = null)
             }
             else
             {
-                QuoteArgument(exe0);
+                if (quoteExe)
+                    QuoteArgument(exe0);
+                else
+                    Argument(exe0);
             }
             Argument(arg0);
             Argument(argumentsOpt);

csharp/autobuilder/Semmle.Autobuild/MsBuildRule.cs

@@ -57,7 +57,14 @@ public BuildScript Analyse(Autobuilder builder, bool auto)
                 var command = new CommandBuilder(builder.Actions);
 
                 if (vsTools != null)
+                {
                     command.CallBatFile(vsTools.Path);
+                    // `vcvarsall.bat` sets a default Platform environment variable,
+                    // which may not be compatible with the supported platforms of the
+                    // given project/solution. Unsetting it means that the default platform
+                    // of the project/solution is used instead.
+                    command.RunCommand("set Platform=&& type NUL", quoteExe: false);
+                }
 
                 builder.MaybeIndex(command, MsBuild);
                 command.QuoteArgument(projectOrSolution.FullPath);

javascript/extractor/src/com/semmle/js/parser/TypeScriptASTConverter.java

@@ -1588,7 +1588,16 @@ private Node convertLabeledStatement(JsonObject node, SourceLocation loc) throws
   }
 
   private Node convertLiteralType(JsonObject node, SourceLocation loc) throws ParseError {
-    return convertChild(node, "literal");
+    Node literal = convertChild(node, "literal");
+    // Convert a negated literal to a negative number
+    if (literal instanceof UnaryExpression) {
+      UnaryExpression unary = (UnaryExpression) literal;
+      if (unary.getOperator().equals("-") && unary.getArgument() instanceof Literal) {
+        Literal arg = (Literal) unary.getArgument();
+        literal = new Literal(loc, arg.getTokenType(), "-" + arg.getValue());
+      }
+    }
+    return literal;
   }
 
   private Node convertMappedType(JsonObject node, SourceLocation loc) throws ParseError {

javascript/ql/src/javascript.qll

@@ -12,6 +12,7 @@ import semmle.javascript.Base64
 import semmle.javascript.CFG
 import semmle.javascript.Classes
 import semmle.javascript.Closure
+import semmle.javascript.Collections
 import semmle.javascript.Comments
 import semmle.javascript.Concepts
 import semmle.javascript.Constants

javascript/ql/src/semmle/javascript/Arrays.qll

@@ -96,10 +96,55 @@ module ArrayTaintTracking {
  * Classes and predicates for modelling data-flow for arrays.
  */
 private module ArrayDataFlow {
+  private import DataFlow::PseudoProperties
+
   /**
-   * Gets a pseudo-field representing an element inside an array.
+   * A step modelling the creation of an Array using the `Array.from(x)` method.
+   * The step copies the elements of the argument (set, array, or iterator elements) into the resulting array.
    */
-  private string arrayElement() { result = "$arrayElement$" }
+  private class ArrayFrom extends DataFlow::AdditionalFlowStep, DataFlow::CallNode {
+    ArrayFrom() { this = DataFlow::globalVarRef("Array").getAMemberCall("from") }
+
+    override predicate loadStoreStep(
+      DataFlow::Node pred, DataFlow::Node succ, string fromProp, string toProp
+    ) {
+      pred = this.getArgument(0) and
+      succ = this and
+      fromProp = arrayLikeElement() and
+      toProp = arrayElement()
+    }
+  }
+
+  /**
+   * A step modelling an array copy where the spread operator is used.
+   * The result is essentially array concatenation.
+   *
+   * Such a step can occur both with the `push` and `unshift` methods, or when creating a new array.
+   */
+  private class ArrayCopySpread extends DataFlow::AdditionalFlowStep {
+    DataFlow::Node spreadArgument; // the spread argument containing the elements to be copied.
+    DataFlow::Node base; // the object where the elements should be copied to.
+
+    ArrayCopySpread() {
+      exists(DataFlow::MethodCallNode mcn | mcn = this |
+        mcn.getMethodName() = ["push", "unshift"] and
+        spreadArgument = mcn.getASpreadArgument() and
+        base = mcn.getReceiver().getALocalSource()
+      )
+      or
+      spreadArgument = this.(DataFlow::ArrayCreationNode).getASpreadArgument() and
+      base = this
+    }
+
+    override predicate loadStoreStep(
+      DataFlow::Node pred, DataFlow::Node succ, string fromProp, string toProp
+    ) {
+      pred = spreadArgument and
+      succ = base and
+      fromProp = arrayLikeElement() and
+      toProp = arrayElement()
+    }
+  }
 
   /**
    * A step for storing an element on an array using `arr.push(e)` or `arr.unshift(e)`.
@@ -110,10 +155,10 @@ private module ArrayDataFlow {
       this.getMethodName() = "unshift"
     }
 
-    override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
+    override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
       prop = arrayElement() and
-      (element = this.getAnArgument() or element = this.getASpreadArgument()) and
-      obj = this.getReceiver().getALocalSource()
+      element = this.getAnArgument() and
+      obj.getAMethodCall() = this
     }
   }
 
@@ -143,10 +188,10 @@ private module ArrayDataFlow {
       element = this
     }
 
-    override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
+    override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
       prop = arrayElement() and
       element = this.(DataFlow::PropWrite).getRhs() and
-      this = obj.(DataFlow::SourceNode).getAPropertyWrite()
+      this = obj.getAPropertyWrite()
     }
   }
 
@@ -189,7 +234,7 @@ private module ArrayDataFlow {
       element = getCallback(0).getParameter(0)
     }
 
-    override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
+    override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
       this.getMethodName() = "map" and
       prop = arrayElement() and
       element = this.getCallback(0).getAReturn() and
@@ -209,7 +254,7 @@ private module ArrayDataFlow {
   private class ArrayCreationStep extends DataFlow::AdditionalFlowStep, DataFlow::Node {
     ArrayCreationStep() { this instanceof DataFlow::ArrayCreationNode }
 
-    override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
+    override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
       prop = arrayElement() and
       element = this.(DataFlow::ArrayCreationNode).getAnElement() and
       obj = this
@@ -223,10 +268,10 @@ private module ArrayDataFlow {
   private class ArraySpliceStep extends DataFlow::AdditionalFlowStep, DataFlow::MethodCallNode {
     ArraySpliceStep() { this.getMethodName() = "splice" }
 
-    override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
+    override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
       prop = arrayElement() and
       element = getArgument(2) and
-      obj = this.getReceiver().getALocalSource()
+      this = obj.getAMethodCall()
     }
   }
 
@@ -260,4 +305,20 @@ private module ArrayDataFlow {
       succ = this
     }
   }
+
+  /**
+   * A step for modelling `for of` iteration on arrays.
+   */
+  private class ForOfStep extends DataFlow::AdditionalFlowStep, DataFlow::ValueNode {
+    ForOfStmt forOf;
+    DataFlow::Node element;
+
+    ForOfStep() { this.asExpr() = forOf.getIterationDomain() }
+
+    override predicate loadStep(DataFlow::Node obj, DataFlow::Node e, string prop) {
+      obj = this and
+      e = DataFlow::lvalueNode(forOf.getLValue()) and
+      prop = arrayElement()
+    }
+  }
 }