Merge pull request #840 from esben-semmle/js/propagate-sound-avalue

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -10,6 +10,8 @@
 
 * The taint tracking library now recognizes flow through persistent storage, class fields, and callbacks in certain cases. This may give more results for the security queries.
 
+* Type inference for function calls has been improved. This may give additional results for queries that rely on type inference.
+
 ## New queries
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |

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

@@ -41,6 +41,27 @@ private class SsaVarAccessAnalysis extends DataFlow::AnalyzedValueNode {
   override AbstractValue getALocalValue() { result = def.getAnRhsValue() }
 }
 
+/**
+ * Flow analysis for accesses to SSA variables.
+ *
+ * Unlike `SsaVarAccessAnalysis`, this only contributes to `getAValue()`, not `getALocalValue()`.
+ */
+private class SsaVarAccessWithNonLocalAnalysis extends SsaVarAccessAnalysis {
+  DataFlow::AnalyzedValueNode src;
+
+  SsaVarAccessWithNonLocalAnalysis() {
+    exists(VarDef varDef |
+      varDef = def.(SsaExplicitDefinition).getDef() and
+      varDef.getSource().flow() = src and
+      src instanceof CallWithNonLocalAnalyzedReturnFlow and
+      // avoid relating `v` and `f()` in `var {v} = f();`
+      not varDef.getTarget() instanceof DestructuringPattern
+    )
+  }
+
+  override AbstractValue getAValue() { result = src.getAValue() }
+}
+
 /**
  * Flow analysis for `VarDef`s.
  */

javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected

@@ -25,6 +25,10 @@
 | UselessConditional.js:129:6:129:24 | constantUndefined() | This call to constantUndefined always evaluates to false. |
 | UselessConditional.js:135:6:135:32 | constan ... ined1() | This call to constantFalseOrUndefined1 always evaluates to false. |
 | UselessConditional.js:139:6:139:32 | constan ... ined2() | This call to constantFalseOrUndefined2 always evaluates to false. |
+| UselessConditional.js:148:6:148:8 | p() | This call to p always evaluates to true. |
+| UselessConditional.js:151:6:151:6 | v | This use of variable 'v' always evaluates to true. |
+| UselessConditional.js:163:5:163:17 | findOrThrow() | This call to findOrThrow always evaluates to true. |
+| UselessConditional.js:166:6:166:6 | v | This use of variable 'v' always evaluates to true. |
 | UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. |
 | UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. |
 | UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. |

javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js

@@ -140,4 +140,41 @@ async function awaitFlow(){
         return;
 
 });
+
+(function () {
+	function p() {
+		return {};
+	}
+	if (p()) { // NOT OK
+	}
+	var v = p();
+	if (v) { // NOT OK
+	}
+	if (v) { // NOT OK, but not detected due to SSA limitations
+	}
+});
+
+(function() {
+	function findOrThrow() {
+		var e = find();
+		if (e) return e;
+		throw new Error();
+	}
+	if(findOrThrow()){ // NOT OK
+	}
+	var v = findOrThrow();
+	if (v) { // NOT OK
+	}
+	if (v) { // NOT OK, but not detected due to SSA limitations
+	}
+});
+
+(function () {
+	function f(){ return { v: unkown };}
+	f();
+	var { v } = f();
+	if (v) { // OK
+	}
+});
+
 // semmle-extractor-options: --experimental