Merge pull request #1355 from xiemaisi/js/data-flow-api-fiddling

Approved by asger-semmle

javascript/ql/src/semmle/javascript/RangeAnalysis.qll

@@ -126,9 +126,7 @@ module RangeAnalysis {
    * the given increment/decrement expression.
    */
   private DataFlow::Node updateExprResult(UpdateExpr expr) {
-    exists(SsaExplicitDefinition def | def.getDef() = expr |
-      result = DataFlow::ssaDefinitionNode(def)
-    )
+    result = DataFlow::ssaDefinitionNode(SSA::definition(expr))
     or
     expr.isPrefix() and
     result = expr.flow()
@@ -138,9 +136,7 @@ module RangeAnalysis {
    * Gets a data flow node holding the result of the given componund assignment.
    */
   private DataFlow::Node compoundAssignResult(CompoundAssignExpr expr) {
-    exists(SsaExplicitDefinition def | def.getDef() = expr |
-      result = DataFlow::ssaDefinitionNode(def)
-    )
+    result = DataFlow::ssaDefinitionNode(SSA::definition(expr))
     or
     result = expr.flow()
   }

javascript/ql/src/semmle/javascript/SSA.qll

@@ -699,3 +699,11 @@ class SsaRefinementNode extends SsaPseudoDefinition, TRefinement {
     getGuard().getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
   }
 }
+
+module SSA {
+  /** Gets the SSA definition corresponding to `d`. */
+  SsaExplicitDefinition definition(VarDef d) { result.getDef() = d }
+
+  /** Gets the SSA variable corresponding to `d`. */
+  SsaVariable variable(VarDef d) { result.getDefinition() = definition(d) }
+}

javascript/ql/src/semmle/javascript/StringConcatenation.qll

@@ -9,9 +9,7 @@ module StringConcatenation {
   private DataFlow::Node getAssignAddResult(AssignAddExpr expr) {
     result = expr.flow()
     or
-    exists(SsaExplicitDefinition def | def.getDef() = expr |
-      result = DataFlow::ssaDefinitionNode(def)
-    )
+    result = DataFlow::ssaDefinitionNode(SSA::definition(expr))
   }
 
   /** Gets the `n`th operand to the string concatenation defining `node`. */

javascript/ql/src/semmle/javascript/Variables.qll

@@ -217,7 +217,7 @@ class ArgumentsVariable extends Variable {
 /** An identifier that refers to a variable, either in a declaration or in a variable access. */
 class VarRef extends @varref, Identifier, BindingPattern, LexicalRef {
   /** Gets the variable this identifier refers to. */
-  Variable getVariable() { none() } // Overriden in VarAccess and VarDecl
+  override Variable getVariable() { none() } // Overriden in VarAccess and VarDecl
 
   override string getName() { result = Identifier.super.getName() }
 
@@ -316,6 +316,9 @@ class BindingPattern extends @pattern, Expr {
   /** Gets the name of this binding pattern if it is an identifier. */
   string getName() { none() }
 
+  /** Gets the variable this binding pattern refers to if it is an identifier. */
+  Variable getVariable() { none() }
+
   /** Gets a variable reference in binding position within this pattern. */
   VarRef getABindingVarRef() { none() }
 

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

@@ -378,11 +378,8 @@ private class FlowStepThroughImport extends AdditionalFlowStep, DataFlow::ValueN
   override ImportSpecifier astNode;
 
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(SsaExplicitDefinition ssa |
-      pred = this and
-      ssa.getDef() = astNode and
-      succ = DataFlow::ssaDefinitionNode(ssa)
-    )
+    pred = this and
+    succ = DataFlow::ssaDefinitionNode(SSA::definition(astNode))
   }
 }
 
@@ -927,9 +924,7 @@ class PathNode extends TPathNode {
   }
 
   /** Gets a successor node of this path node. */
-  PathNode getASuccessor() {
-    result = getASuccessorInternal().getAHiddenSuccessor*()
-  }
+  PathNode getASuccessor() { result = getASuccessorInternal().getAHiddenSuccessor*() }
 
   /** Gets a textual representation of this path node. */
   string toString() { result = nd.toString() }
@@ -953,7 +948,8 @@ class PathNode extends TPathNode {
    */
   predicate isHidden() {
     // Skip phi, refinement, and capture nodes
-    nd.(DataFlow::SsaDefinitionNode).getSsaVariable().getDefinition() instanceof SsaImplicitDefinition
+    nd.(DataFlow::SsaDefinitionNode).getSsaVariable().getDefinition() instanceof
+      SsaImplicitDefinition
     or
     // Skip to the top of big left-leaning string concatenation trees.
     nd = any(AddExpr add).flow() and

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

@@ -36,7 +36,7 @@ module DataFlow {
     } or
     TThisNode(StmtContainer f) { f.(Function).getThisBinder() = f or f instanceof TopLevel } or
     TUnusedParameterNode(SimpleParameter p) {
-      not exists(SsaExplicitDefinition ssa | p = ssa.getDef())
+      not exists(SSA::definition(p))
     } or
     TDestructuredModuleImportNode(ImportDeclaration decl) {
       exists(decl.getASpecifier().getImportedName())
@@ -718,10 +718,7 @@ module DataFlow {
     ImportSpecifierAsPropRead() {
       spec = imprt.getASpecifier() and
       exists(spec.getImportedName()) and
-      exists(SsaExplicitDefinition ssa |
-        ssa.getDef() = spec and
-        this = TSsaDefNode(ssa)
-      )
+      this = ssaDefinitionNode(SSA::definition(spec))
     }
 
     override Node getBase() { result = TDestructuredModuleImportNode(imprt) }
@@ -980,6 +977,11 @@ module DataFlow {
    */
   ValueNode valueNode(ASTNode nd) { result.getAstNode() = nd }
 
+  /**
+   * Gets the data flow node corresponding to `e`.
+   */
+  ExprNode exprNode(Expr e) { result = valueNode(e) }
+
   /** Gets the data flow node corresponding to `ssa`. */
   SsaDefinitionNode ssaDefinitionNode(SsaDefinition ssa) { result = TSsaDefNode(ssa) }
 
@@ -990,11 +992,7 @@ module DataFlow {
    * INTERNAL: Use `parameterNode(Parameter)` instead.
    */
   predicate parameterNode(DataFlow::Node nd, Parameter p) {
-    exists(SsaExplicitDefinition ssa |
-      nd = ssaDefinitionNode(ssa) and
-      p = ssa.getDef() and
-      p instanceof SimpleParameter
-    )
+    nd = ssaDefinitionNode(SSA::definition((SimpleParameter)p))
     or
     nd = TDestructuringPatternNode(p)
     or

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

@@ -6,6 +6,11 @@
 
 import javascript
 
+/** A data flow node corresponding to an expression. */
+class ExprNode extends DataFlow::ValueNode {
+  override Expr astNode;
+}
+
 /** A data flow node corresponding to a parameter. */
 class ParameterNode extends DataFlow::SourceNode {
   Parameter p;
@@ -467,11 +472,10 @@ module ModuleImportNode {
       )
       or
       // `import * as http from 'http'` or `import http from `http`'
-      exists(ImportDeclaration id, ImportSpecifier is, SsaExplicitDefinition ssa |
+      exists(ImportDeclaration id, ImportSpecifier is |
         id.getImportedPath().getValue() = path and
         is = id.getASpecifier() and
-        ssa.getDef() = is and
-        this = DataFlow::ssaDefinitionNode(ssa)
+        this = DataFlow::ssaDefinitionNode(SSA::definition(is))
       |
         is instanceof ImportNamespaceSpecifier and
         count(id.getASpecifier()) = 1

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

@@ -223,11 +223,7 @@ module SourceNode {
         astNode instanceof RegExpLiteral
       )
       or
-      exists(SsaExplicitDefinition ssa, VarDef def |
-        this = DataFlow::ssaDefinitionNode(ssa) and def = ssa.getDef()
-      |
-        def instanceof ImportSpecifier
-      )
+      this = DataFlow::ssaDefinitionNode(SSA::definition(any(ImportSpecifier imp)))
       or
       DataFlow::parameterNode(this, _)
       or

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

@@ -198,11 +198,10 @@ module TaintTracking {
       succ.(DataFlow::PropRead).getBase() = pred
       or
       // iterating over a tainted iterator taints the loop variable
-      exists(EnhancedForLoop efl, SsaExplicitDefinition ssa |
+      exists(EnhancedForLoop efl |
         this = DataFlow::valueNode(efl.getIterationDomain()) and
         pred = this and
-        ssa.getDef() = efl.getIteratorExpr() and
-        succ = DataFlow::ssaDefinitionNode(ssa)
+        succ = DataFlow::ssaDefinitionNode(SSA::definition(efl.getIteratorExpr()))
       )
     }
   }

javascript/ql/src/semmle/javascript/security/SensitiveActions.qll

@@ -165,11 +165,7 @@ private class BasicSensitiveWrite extends SensitiveWrite {
       exists(VarDef v | v.getAVariable().getName() = name |
         if exists(v.getSource())
         then v.getSource() = this.asExpr()
-        else
-          exists(SsaExplicitDefinition ssa |
-            DataFlow::ssaDefinitionNode(ssa) = this and
-            ssa.getDef() = v
-          )
+        else this = DataFlow::ssaDefinitionNode(SSA::definition(v))
       )
     )
   }