Merge pull request #833 from esben-semmle/js/sharpen-cond

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -37,6 +37,7 @@
 | Useless assignment to property. | Fewer false-positive results | This rule now treats assignments with complex right-hand sides correctly. |
 | Unsafe dynamic method access              | Fewer false-positive results | This rule no longer flags concatenated strings as unsafe method names. |
 | Unvalidated dynamic method call           | More true-positive results | This rule now flags concatenated strings as unvalidated method names in more cases. |
+| Useless conditional | More true-positive results | This rule now flags additional uses of function call values. | 
 
 ## Changes to QL libraries
 

javascript/ql/src/Statements/UselessConditional.ql

@@ -65,17 +65,24 @@ predicate isInitialParameterUse(Expr e) {
 }
 
 /**
- * Holds if `e` directly uses the returned value from a function call that returns a constant boolean value.
+ * Holds if `e` directly uses the returned value from functions that return constant boolean values.
  */
 predicate isConstantBooleanReturnValue(Expr e) {
   // unlike `SourceNode.flowsTo` this will not include uses we have refinement information for
-  exists(DataFlow::CallNode call | exists(call.analyze().getTheBooleanValue()) |
-    e = call.asExpr()
-    or
-    // also support return values that are assigned to variables
-    exists(SsaExplicitDefinition ssa |
-      ssa.getDef().getSource() = call.asExpr() and
-      ssa.getVariable().getAUse() = e
+  exists(string b | (b = "true" or b = "false") |
+    forex(DataFlow::CallNode call, Expr ret |
+      ret = call.getACallee().getAReturnedExpr() and
+      (
+        e = call.asExpr()
+        or
+        // also support return values that are assigned to variables
+        exists(SsaExplicitDefinition ssa |
+          ssa.getDef().getSource() = call.asExpr() and
+          ssa.getVariable().getAUse() = e
+        )
+      )
+      |
+      ret.(BooleanLiteral).getValue() = b
     )
   )
   or

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

@@ -22,6 +22,9 @@
 | UselessConditional.js:102:19:102:19 | x | This use of variable 'x' always evaluates to false. |
 | UselessConditional.js:103:23:103:23 | x | This use of variable 'x' always evaluates to false. |
 | UselessConditional.js:109:15:109:16 | {} | This expression always evaluates to true. |
+| 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. |
 | 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

@@ -109,4 +109,35 @@ async function awaitFlow(){
     if ((x && {}) || y) {} // NOT OK
 });
 
+(function(){
+    function constantFalse1() {
+        return false;
+    }
+    if (constantFalse1()) // OK
+        return;
+
+	function constantFalse2() {
+		return false;
+    }
+	let constantFalse = unknown? constantFalse1 : constantFalse2;
+    if (constantFalse2()) // OK
+        return;
+
+	function constantUndefined() {
+        return undefined;
+    }
+	if (constantUndefined()) // NOT OK
+        return;
+
+	function constantFalseOrUndefined1() {
+		return unknown? false: undefined;
+	}
+	if (constantFalseOrUndefined1()) // NOT OK
+        return;
+
+	let constantFalseOrUndefined2 = unknown? constantFalse1 : constantUndefined;
+	if (constantFalseOrUndefined2())  // NOT OK
+        return;
+
+});
 // semmle-extractor-options: --experimental