JS(ql): support optional chaining

Esben Sparre Andreasen · Esben Sparre Andreasen · commit 41b45352aa3a · 2018-11-21T08:57:10.000+01:00
diff --git a/javascript/ql/src/Expressions/SuspiciousInvocation.ql b/javascript/ql/src/Expressions/SuspiciousInvocation.ql
@@ -16,5 +16,6 @@ private import semmle.javascript.dataflow.InferredTypes
 from InvokeExpr invk, DataFlow::AnalyzedNode callee
 where callee.asExpr() = invk.getCallee() and
       forex (InferredType tp | tp = callee.getAType() | tp != TTFunction() and tp != TTClass()) and
-      not invk.isAmbient()
+      not invk.isAmbient() and
+      not invk instanceof OptionalUse
 select invk, "Callee is not a function: it has type " + callee.ppTypes() + "."
diff --git a/javascript/ql/src/Expressions/SuspiciousPropAccess.ql b/javascript/ql/src/Expressions/SuspiciousPropAccess.ql
@@ -32,5 +32,6 @@ from PropAccess pacc, DataFlow::AnalyzedNode base
 where base.asExpr() = pacc.getBase() and
       forex (InferredType tp | tp = base.getAType() | tp = TTNull() or tp = TTUndefined()) and
       not namespaceOrConstEnumAccess(pacc.getBase()) and
-      not pacc.isAmbient()
+      not pacc.isAmbient() and
+      not pacc instanceof OptionalUse
 select pacc, "The base expression of this property access is always " + base.ppTypes() + "."
diff --git a/javascript/ql/src/semmle/javascript/Expr.qll b/javascript/ql/src/semmle/javascript/Expr.qll
@@ -1901,4 +1901,36 @@ private class LiteralDynamicImportPath extends PathExprInModule, ConstantString
   }
 
   override string getValue() { result = this.(ConstantString).getStringValue() }
-}
+}
+
+/**
+ * A call or member access that evaluates to `undefined` if its base operand evaluates to `undefined` or `null`.
+ */
+class OptionalUse extends Expr, @optionalchainable { OptionalUse() { isOptionalChaining(this) } }
+
+private class ChainElem extends Expr, @optionalchainable {
+  /**
+   * Gets the base operand of this chainable element.
+   */
+  ChainElem getChainBase() {
+    result = this.(CallExpr).getCallee() or
+    result = this.(PropAccess).getBase()
+  }
+}
+
+/**
+ * The root in a chain of calls or property accesses, where at least one call or property access is optional.
+ */
+class OptionalChainRoot extends ChainElem {
+  OptionalUse optionalUse;
+
+  OptionalChainRoot() {
+    getChainBase*() = optionalUse and
+    not exists(ChainElem other | this = other.getChainBase())
+  }
+
+  /**
+   * Gets an optional call or property access in the chain of this root.
+   */
+  OptionalUse getAnOptionalUse() { result = optionalUse }
+}
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/BasicExprTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/BasicExprTypeInference.qll
@@ -413,3 +413,15 @@ private class AnalyzedAssignAddExpr extends AnalyzedCompoundAssignExpr {
     isAddition(astNode) and result = abstractValueOfType(TTNumber())
   }
 }
+
+/**
+ * Flow analysis for optional chaining expressions.
+ */
+private class AnalyzedOptionalChainExpr extends DataFlow::AnalyzedValueNode {
+  override OptionalChainRoot astNode;
+
+  override AbstractValue getALocalValue() {
+    result = super.getALocalValue() or
+    result = TAbstractUndefined()
+  }
+}
diff --git a/javascript/ql/test/library-tests/OptionalChaining/OptionalChainRoot.expected b/javascript/ql/test/library-tests/OptionalChaining/OptionalChainRoot.expected
@@ -0,0 +1,18 @@
+| short-circuiting.js:3:5:3:18 | x?.(o1 = null) | short-circuiting.js:3:5:3:18 | x?.(o1 = null) |
+| short-circuiting.js:7:5:7:18 | x?.[o2 = null] | short-circuiting.js:7:5:7:18 | x?.[o2 = null] |
+| short-circuiting.js:12:5:12:31 | x?.[o3  ... = null) | short-circuiting.js:12:5:12:18 | x?.[o3 = null] |
+| short-circuiting.js:12:5:12:31 | x?.[o3  ... = null) | short-circuiting.js:12:5:12:31 | x?.[o3  ... = null) |
+| tst.js:2:1:2:6 | a?.b.c | tst.js:2:1:2:4 | a?.b |
+| tst.js:3:1:3:6 | a.b?.c | tst.js:3:1:3:6 | a.b?.c |
+| tst.js:4:1:4:7 | a?.b?.c | tst.js:4:1:4:4 | a?.b |
+| tst.js:4:1:4:7 | a?.b?.c | tst.js:4:1:4:7 | a?.b?.c |
+| tst.js:7:1:7:7 | f?.()() | tst.js:7:1:7:5 | f?.() |
+| tst.js:8:1:8:7 | f()?.() | tst.js:8:1:8:7 | f()?.() |
+| tst.js:9:1:9:9 | f?.()?.() | tst.js:9:1:9:5 | f?.() |
+| tst.js:9:1:9:9 | f?.()?.() | tst.js:9:1:9:9 | f?.()?.() |
+| tst.js:12:1:12:8 | a?.m().b | tst.js:12:1:12:4 | a?.m |
+| tst.js:13:1:13:9 | a.m?.().b | tst.js:13:1:13:7 | a.m?.() |
+| tst.js:14:1:14:8 | a.m()?.b | tst.js:14:1:14:8 | a.m()?.b |
+| tst.js:15:1:15:11 | a?.m?.()?.b | tst.js:15:1:15:4 | a?.m |
+| tst.js:15:1:15:11 | a?.m?.()?.b | tst.js:15:1:15:8 | a?.m?.() |
+| tst.js:15:1:15:11 | a?.m?.()?.b | tst.js:15:1:15:11 | a?.m?.()?.b |
diff --git a/javascript/ql/test/library-tests/OptionalChaining/OptionalChainRoot.ql b/javascript/ql/test/library-tests/OptionalChaining/OptionalChainRoot.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from OptionalChainRoot root
+select root, root.getAnOptionalUse()
diff --git a/javascript/ql/test/library-tests/OptionalChaining/OptionalUse.expected b/javascript/ql/test/library-tests/OptionalChaining/OptionalUse.expected
@@ -0,0 +1,18 @@
+| short-circuiting.js:3:5:3:18 | x?.(o1 = null) |
+| short-circuiting.js:7:5:7:18 | x?.[o2 = null] |
+| short-circuiting.js:12:5:12:18 | x?.[o3 = null] |
+| short-circuiting.js:12:5:12:31 | x?.[o3  ... = null) |
+| tst.js:2:1:2:4 | a?.b |
+| tst.js:3:1:3:6 | a.b?.c |
+| tst.js:4:1:4:4 | a?.b |
+| tst.js:4:1:4:7 | a?.b?.c |
+| tst.js:7:1:7:5 | f?.() |
+| tst.js:8:1:8:7 | f()?.() |
+| tst.js:9:1:9:5 | f?.() |
+| tst.js:9:1:9:9 | f?.()?.() |
+| tst.js:12:1:12:4 | a?.m |
+| tst.js:13:1:13:7 | a.m?.() |
+| tst.js:14:1:14:8 | a.m()?.b |
+| tst.js:15:1:15:4 | a?.m |
+| tst.js:15:1:15:8 | a?.m?.() |
+| tst.js:15:1:15:11 | a?.m?.()?.b |
diff --git a/javascript/ql/test/library-tests/OptionalChaining/OptionalUse.ql b/javascript/ql/test/library-tests/OptionalChaining/OptionalUse.ql
@@ -0,0 +1,3 @@
+import javascript
+
+select any(OptionalUse u)
diff --git a/javascript/ql/test/library-tests/OptionalChaining/ShortCircuiting.expected b/javascript/ql/test/library-tests/OptionalChaining/ShortCircuiting.expected
@@ -0,0 +1,8 @@
+| short-circuiting.js:4:10:4:11 | o1 | file://:0:0:0:0 | null |
+| short-circuiting.js:4:10:4:11 | o1 | short-circuiting.js:2:14:2:15 | object literal |
+| short-circuiting.js:8:10:8:11 | o2 | file://:0:0:0:0 | null |
+| short-circuiting.js:8:10:8:11 | o2 | short-circuiting.js:6:14:6:15 | object literal |
+| short-circuiting.js:13:10:13:11 | o3 | file://:0:0:0:0 | null |
+| short-circuiting.js:13:10:13:11 | o3 | short-circuiting.js:10:14:10:15 | object literal |
+| short-circuiting.js:14:10:14:11 | o4 | file://:0:0:0:0 | null |
+| short-circuiting.js:14:10:14:11 | o4 | short-circuiting.js:11:14:11:15 | object literal |
diff --git a/javascript/ql/test/library-tests/OptionalChaining/ShortCircuiting.ql b/javascript/ql/test/library-tests/OptionalChaining/ShortCircuiting.ql
@@ -0,0 +1,7 @@
+import javascript
+
+from CallExpr c, Expr arg
+where
+      c.getCalleeName() = "DUMP" and
+      arg = c.getArgument(0)
+select arg, arg.analyze().getAValue()
diff --git a/javascript/ql/test/library-tests/OptionalChaining/ShortCirtuiting.expected b/javascript/ql/test/library-tests/OptionalChaining/ShortCirtuiting.expected
@@ -0,0 +1,8 @@
+| short-circuiting.js:4:10:4:11 | o1 | file://:0:0:0:0 | null |
+| short-circuiting.js:4:10:4:11 | o1 | short-circuiting.js:2:14:2:15 | object literal |
+| short-circuiting.js:8:10:8:11 | o2 | file://:0:0:0:0 | null |
+| short-circuiting.js:8:10:8:11 | o2 | short-circuiting.js:6:14:6:15 | object literal |
+| short-circuiting.js:13:10:13:11 | o3 | file://:0:0:0:0 | null |
+| short-circuiting.js:13:10:13:11 | o3 | short-circuiting.js:10:14:10:15 | object literal |
+| short-circuiting.js:14:10:14:11 | o4 | file://:0:0:0:0 | null |
+| short-circuiting.js:14:10:14:11 | o4 | short-circuiting.js:11:14:11:15 | object literal |
diff --git a/javascript/ql/test/library-tests/OptionalChaining/ShortCirtuiting.ql b/javascript/ql/test/library-tests/OptionalChaining/ShortCirtuiting.ql
@@ -0,0 +1,7 @@
+import javascript
+
+from CallExpr c, Expr arg
+where
+      c.getCalleeName() = "DUMP" and
+      arg = c.getArgument(0)
+select arg, arg.analyze().getAValue()
diff --git a/javascript/ql/test/library-tests/OptionalChaining/short-circuiting.js b/javascript/ql/test/library-tests/OptionalChaining/short-circuiting.js
@@ -0,0 +1,16 @@
+(function() {
+    var o1 = {};
+    x?.(o1 = null);
+    DUMP(o1);
+
+    var o2 = {};
+    x?.[o2 = null];
+    DUMP(o2);
+
+    var o3 = {},
+        o4 = {};
+    x?.[o3 = null]?.(o4 = null);
+    DUMP(o3);
+    DUMP(o4);
+});
+// semmle-extractor-options: --experimental
diff --git a/javascript/ql/test/library-tests/OptionalChaining/tst.js b/javascript/ql/test/library-tests/OptionalChaining/tst.js
@@ -0,0 +1,17 @@
+a.b.c;
+a?.b.c;
+a.b?.c;
+a?.b?.c;
+
+f()();
+f?.()();
+f()?.();
+f?.()?.();
+
+a.m().b;
+a?.m().b;
+a.m?.().b;
+a.m()?.b;
+a?.m?.()?.b;
+
+// semmle-extractor-options: --experimental
diff --git a/javascript/ql/test/library-tests/TypeInference/OptionalChaining/test.expected b/javascript/ql/test/library-tests/TypeInference/OptionalChaining/test.expected
@@ -0,0 +1,14 @@
+| tst.js:2:14:2:21 | (null)() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:3:14:3:23 | (null)?.() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:3:14:3:23 | (null)?.() | file://:0:0:0:0 | undefined |
+| tst.js:4:14:4:26 | (undefined)() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:5:14:5:28 | (undefined)?.() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:5:14:5:28 | (undefined)?.() | file://:0:0:0:0 | undefined |
+| tst.js:6:14:6:24 | (unknown)() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:7:14:7:26 | (unknown)?.() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:7:14:7:26 | (unknown)?.() | file://:0:0:0:0 | undefined |
+| tst.js:9:13:11:5 | unknown ... ;\\n    } | file://:0:0:0:0 | undefined |
+| tst.js:9:13:11:5 | unknown ... ;\\n    } | tst.js:9:33:11:5 | anonymous function |
+| tst.js:12:14:12:16 | f() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:13:14:13:18 | f?.() | file://:0:0:0:0 | indefinite value (call) |
+| tst.js:13:14:13:18 | f?.() | file://:0:0:0:0 | undefined |
diff --git a/javascript/ql/test/library-tests/TypeInference/OptionalChaining/test.ql b/javascript/ql/test/library-tests/TypeInference/OptionalChaining/test.ql
@@ -0,0 +1,5 @@
+import javascript
+
+from Variable v, Expr e
+where e = v.getAnAssignedExpr()
+select e, e.analyze().getAValue()
diff --git a/javascript/ql/test/library-tests/TypeInference/OptionalChaining/tst.js b/javascript/ql/test/library-tests/TypeInference/OptionalChaining/tst.js
@@ -0,0 +1,15 @@
+(function() {
+    var v1 = (null)();
+    var v2 = (null)?.();
+    var v3 = (undefined)();
+    var v4 = (undefined)?.();
+    var v5 = (unknown)();
+    var v6 = (unknown)?.();
+
+    var f = unknown? undefined: function(){
+        return 42;
+    }
+    var v7 = f();
+    var v8 = f?.();
+});
+// semmle-extractor-options: --experimental
diff --git a/javascript/ql/test/query-tests/Expressions/SuspiciousInvocation/SuspiciousInvocation.expected b/javascript/ql/test/query-tests/Expressions/SuspiciousInvocation/SuspiciousInvocation.expected
@@ -1,3 +1,5 @@
 | SuspiciousInvocation.js:11:5:11:58 | error(" ... status) | Callee is not a function: it has type undefined. |
 | namespace.ts:23:1:23:3 | g() | Callee is not a function: it has type object. |
+| optional-chaining.js:3:5:3:7 | a() | Callee is not a function: it has type null. |
+| optional-chaining.js:7:5:7:7 | b() | Callee is not a function: it has type undefined. |
 | super.js:11:5:11:11 | super() | Callee is not a function: it has type number. |
diff --git a/javascript/ql/test/query-tests/Expressions/SuspiciousInvocation/optional-chaining.js b/javascript/ql/test/query-tests/Expressions/SuspiciousInvocation/optional-chaining.js
@@ -0,0 +1,10 @@
+(function(){
+    var a = null;
+    a();
+    a?.();
+
+    var b = undefined;
+    b();
+    b?.();
+});
+// semmle-extractor-options: --experimental
diff --git a/javascript/ql/test/query-tests/Expressions/SuspiciousPropAccess/SuspiciousPropAccess.expected b/javascript/ql/test/query-tests/Expressions/SuspiciousPropAccess/SuspiciousPropAccess.expected
@@ -1,4 +1,6 @@
 | SuspiciousPropAccess.js:4:10:4:21 | result.value | The base expression of this property access is always undefined. |
+| optional-chaining.js:3:5:3:7 | a.p | The base expression of this property access is always null. |
+| optional-chaining.js:7:5:7:7 | b.p | The base expression of this property access is always undefined. |
 | tst.js:32:32:32:38 | a(1)[0] | The base expression of this property access is always null. |
 | tst.ts:19:3:19:5 | x.p | The base expression of this property access is always undefined. |
 | typeassertion.ts:14:3:14:9 | z.field | The base expression of this property access is always null. |
diff --git a/javascript/ql/test/query-tests/Expressions/SuspiciousPropAccess/optional-chaining.js b/javascript/ql/test/query-tests/Expressions/SuspiciousPropAccess/optional-chaining.js
@@ -0,0 +1,10 @@
+(function(){
+    var a = null;
+    a.p;
+    a?.p;
+
+    var b = undefined;
+    b.p;
+    b?.p;
+});
+// semmle-extractor-options: --experimental

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+import javascript`
	`2`	`+`
	`3`	`+select any(OptionalUse u)`