Merge pull request #1873 from asger-semmle/type-inf-consistency

Approved by xiemaisi

Author: semmle-qlci

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

@@ -31,33 +31,50 @@ private class AnalyzedCapturedVariable extends @variable {
 }
 
 /**
- * Flow analysis for accesses to SSA variables.
+ * Flow analysis for SSA nodes.
  */
-private class SsaVarAccessAnalysis extends DataFlow::AnalyzedValueNode {
-  AnalyzedSsaDefinition def;
+private class AnalyzedSsaDefinitionNode extends AnalyzedNode, DataFlow::SsaDefinitionNode {
+  override AbstractValue getALocalValue() { result = ssa.(AnalyzedSsaDefinition).getAnRhsValue() }
+}
 
-  SsaVarAccessAnalysis() { astNode = def.getVariable().getAUse() }
+/**
+ * An SSA definition whose right-hand side is a call with non-local data flow.
+ */
+private class SsaDefinitionWithNonLocalFlow extends SsaExplicitDefinition {
+  CallWithNonLocalAnalyzedReturnFlow source;
+
+  SsaDefinitionWithNonLocalFlow() {
+    source = getDef().getSource().flow()
+  }
 
-  override AbstractValue getALocalValue() { result = def.getAnRhsValue() }
+  CallWithNonLocalAnalyzedReturnFlow getSource() { result = source }
 }
 
 /**
- * Flow analysis for accesses to SSA variables.
- *
- * Unlike `SsaVarAccessAnalysis`, this only contributes to `getAValue()`, not `getALocalValue()`.
+ * Flow analysis for SSA nodes corresponding to `SsaDefinitionWithNonLocalFlow`.
  */
-private class SsaVarAccessWithNonLocalAnalysis extends SsaVarAccessAnalysis {
-  DataFlow::AnalyzedValueNode src;
+private class AnalyzedSsaDefinitionNodeWithNonLocalAnalysis extends AnalyzedSsaDefinitionNode {
+  override SsaDefinitionWithNonLocalFlow ssa;
 
-  SsaVarAccessWithNonLocalAnalysis() {
-    exists(VarDef varDef |
-      varDef = def.(SsaExplicitDefinition).getDef() and
-      varDef.getSource().flow() = src and
-      src instanceof CallWithNonLocalAnalyzedReturnFlow
-    )
+  override AbstractValue getAValue() {
+    result = ssa.getSource().getAValue()
+  }
+}
+
+/**
+ * Flow analysis for uses of an SSA variable corresponding to `SsaDefinitionWithNonLocalFlow`.
+ */
+private class AnalyzedSsaVariableUseWithNonLocalFlow extends AnalyzedValueNode {
+  SsaDefinitionWithNonLocalFlow ssaDef;
+
+  AnalyzedSsaVariableUseWithNonLocalFlow() {
+    this = DataFlow::valueNode(ssaDef.getVariable().getAUse())
   }
 
-  override AbstractValue getAValue() { result = src.getAValue() }
+  override AbstractValue getAValue() {
+    // Block indefinite values coming from getALocalValue()
+    result = ssaDef.getSource().getAValue()
+  }
 }
 
 /**
@@ -620,7 +637,11 @@ private class ReflectiveVarFlow extends DataFlow::AnalyzedValueNode {
     )
   }
 
-  override AbstractValue getALocalValue() { result = TIndefiniteAbstractValue("eval") }
+  override AbstractValue getALocalValue() {
+    result = TIndefiniteAbstractValue("eval")
+    or
+    result = AnalyzedValueNode.super.getALocalValue()
+  }
 }
 
 /**
@@ -632,7 +653,11 @@ private class ReflectiveVarFlow extends DataFlow::AnalyzedValueNode {
 private class NamespaceExportVarFlow extends DataFlow::AnalyzedValueNode {
   NamespaceExportVarFlow() { astNode.(VarAccess).getVariable().isNamespaceExport() }
 
-  override AbstractValue getALocalValue() { result = TIndefiniteAbstractValue("namespace") }
+  override AbstractValue getALocalValue() {
+    result = TIndefiniteAbstractValue("namespace")
+    or
+    result = AnalyzedValueNode.super.getALocalValue()
+  }
 }
 
 /**

javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected

@@ -36,8 +36,11 @@ test_isUncertain
 | tst.js:69:1:69:10 | globalfn() |
 test_getAFunctionValue
 | a.js:1:8:1:10 | foo | b.js:1:16:1:27 | function(){} |
+| a.js:1:8:1:10 | foo | b.js:1:16:1:27 | function(){} |
+| a.js:1:15:1:17 | bar | b.js:2:8:2:24 | function bar() {} |
 | a.js:1:15:1:17 | bar | b.js:2:8:2:24 | function bar() {} |
 | a.js:1:20:1:22 | qux | c.js:2:8:2:24 | function bar() {} |
+| a.js:1:20:1:22 | qux | c.js:2:8:2:24 | function bar() {} |
 | a.js:2:1:2:3 | foo | b.js:1:16:1:27 | function(){} |
 | a.js:3:1:3:3 | bar | b.js:2:8:2:24 | function bar() {} |
 | a.js:4:1:4:3 | qux | c.js:2:8:2:24 | function bar() {} |

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -1,8 +1,4 @@
 typeInferenceMismatch
-| addexpr.js:4:10:4:17 | source() | addexpr.js:4:5:4:17 | x |
-| addexpr.js:4:10:4:17 | source() | addexpr.js:6:3:6:14 | x |
-| addexpr.js:11:15:11:22 | source() | addexpr.js:17:5:17:18 | value |
-| addexpr.js:11:15:11:22 | source() | addexpr.js:19:3:19:14 | value |
 | destruct.js:20:7:20:14 | source() | destruct.js:13:14:13:19 | [a, b] |
 #select
 | access-path-sanitizer.js:2:18:2:25 | source() | access-path-sanitizer.js:4:8:4:12 | obj.x |