Merge pull request #1788 from asger-semmle/additional-type-tracking-step

semmle-qlci · web-flow · commit fc59dd6819f7 · 2019-08-24T11:55:16.000+01:00
Approved by xiemaisi
diff --git a/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TypeTracking.qll
@@ -86,6 +86,9 @@ module StepSummary {
       basicLoadStep(pred, succ, prop) and
       summary = LoadStep(prop)
     )
+    or
+    any(AdditionalTypeTrackingStep st).step(pred, succ) and
+    summary = LevelStep()
   }
 }
 
@@ -370,3 +373,20 @@ module TypeBackTracker {
    */
   TypeBackTracker end() { result.end() }
 }
+
+/**
+ * A data flow edge that should be followed by type tracking.
+ *
+ * Unlike `AdditionalFlowStep`, this type of edge does not affect
+ * the local data flow graph, and is not used by data-flow configurations.
+ *
+ * Note: For performance reasons, all subclasses of this class should be part
+ * of the standard library. For query-specific steps, consider including the
+ * custom steps in the type-tracking predicate itself.
+ */
+abstract class AdditionalTypeTrackingStep extends DataFlow::Node {
+  /**
+   * Holds if type-tracking should step from `pred` to `succ`.
+   */
+  abstract predicate step(DataFlow::Node pred, DataFlow::Node succ);
+}
diff --git a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected
@@ -1,26 +1,30 @@
 test_ApiObject
-| tst.js:3:11:3:21 | new myapi() |
-| tst.js:15:10:15:21 | api.chain1() |
-| tst.js:15:10:15:30 | api.cha ... hain2() |
+| tst.js:4:11:4:21 | new myapi() |
+| tst.js:16:10:16:21 | api.chain1() |
+| tst.js:16:10:16:30 | api.cha ... hain2() |
 test_Connection
-| tst.js:6:15:6:18 | conn |
-| tst.js:10:5:10:19 | this.connection |
-| tst.js:15:10:15:49 | api.cha ... ction() |
-| tst.js:18:7:18:21 | getConnection() |
-| tst.js:30:9:30:23 | getConnection() |
-| tst.js:39:7:39:21 | getConnection() |
-| tst.js:47:7:47:21 | getConnection() |
+| tst.js:7:15:7:18 | conn |
+| tst.js:11:5:11:19 | this.connection |
+| tst.js:16:10:16:49 | api.cha ... ction() |
+| tst.js:19:7:19:21 | getConnection() |
+| tst.js:31:9:31:23 | getConnection() |
+| tst.js:40:7:40:21 | getConnection() |
+| tst.js:48:7:48:21 | getConnection() |
+| tst.js:54:37:54:51 | getConnection() |
+| tst.js:57:14:57:48 | config. ... ction') |
 test_DataCallback
-| tst.js:9:11:9:12 | cb |
-| tst.js:20:1:22:1 | functio ... ata);\\n} |
-| tst.js:29:26:29:27 | cb |
-| tst.js:32:17:32:26 | data => {} |
-| tst.js:37:10:37:19 | data => {} |
-| tst.js:39:32:39:45 | getDataCurry() |
-| tst.js:44:19:44:20 | cb |
-| tst.js:47:32:47:60 | identit ... llback) |
+| tst.js:10:11:10:12 | cb |
+| tst.js:21:1:23:1 | functio ... ata);\\n} |
+| tst.js:30:26:30:27 | cb |
+| tst.js:33:17:33:26 | data => {} |
+| tst.js:38:10:38:19 | data => {} |
+| tst.js:40:32:40:45 | getDataCurry() |
+| tst.js:45:19:45:20 | cb |
+| tst.js:48:32:48:60 | identit ... llback) |
+| tst.js:58:16:58:22 | x => {} |
 test_DataValue
-| tst.js:20:18:20:21 | data |
-| tst.js:24:19:24:22 | data |
-| tst.js:32:17:32:20 | data |
-| tst.js:37:10:37:13 | data |
+| tst.js:21:18:21:21 | data |
+| tst.js:25:19:25:22 | data |
+| tst.js:33:17:33:20 | data |
+| tst.js:38:10:38:13 | data |
+| tst.js:58:16:58:16 | x |
diff --git a/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql b/javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql
@@ -1,4 +1,5 @@
 import javascript
+import CustomStep
 
 string chainableMethod() {
   result = "chain1" or
diff --git a/javascript/ql/test/library-tests/TypeTracking/CustomStep.qll b/javascript/ql/test/library-tests/TypeTracking/CustomStep.qll
@@ -0,0 +1,19 @@
+import javascript
+private import DataFlow
+
+predicate configStep(Node pred, Node succ) {
+  exists(CallNode setter, CallNode getter |
+    getter = moduleMember("@test/myconfig", "getConfigValue").getACall() and
+    setter = moduleMember("@test/myconfig", "setConfigValue").getACall() and
+    getter.getArgument(0).getStringValue() = setter.getArgument(0).getStringValue() and
+    pred = setter.getArgument(1) and
+    succ = getter
+  )
+}
+
+class CustomStep extends AdditionalTypeTrackingStep, Node {
+  override predicate step(Node pred, Node succ) {
+    pred = this and
+    configStep(pred, succ)
+  }
+}
diff --git a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected
@@ -1,27 +1,31 @@
 apiObject
-| tst.js:3:11:3:21 | new myapi() |
-| tst.js:15:10:15:21 | api.chain1() |
-| tst.js:15:10:15:30 | api.cha ... hain2() |
+| tst.js:4:11:4:21 | new myapi() |
+| tst.js:16:10:16:21 | api.chain1() |
+| tst.js:16:10:16:30 | api.cha ... hain2() |
 connection
-| type tracker with call steps | tst.js:6:15:6:18 | conn |
-| type tracker with call steps | tst.js:10:5:10:19 | this.connection |
-| type tracker with call steps with property connection | tst.js:6:14:6:13 | this |
-| type tracker without call steps | tst.js:15:10:15:49 | api.cha ... ction() |
-| type tracker without call steps | tst.js:18:7:18:21 | getConnection() |
-| type tracker without call steps | tst.js:30:9:30:23 | getConnection() |
-| type tracker without call steps | tst.js:39:7:39:21 | getConnection() |
-| type tracker without call steps | tst.js:47:7:47:21 | getConnection() |
+| type tracker with call steps | tst.js:7:15:7:18 | conn |
+| type tracker with call steps | tst.js:11:5:11:19 | this.connection |
+| type tracker with call steps with property connection | tst.js:7:14:7:13 | this |
+| type tracker without call steps | tst.js:16:10:16:49 | api.cha ... ction() |
+| type tracker without call steps | tst.js:19:7:19:21 | getConnection() |
+| type tracker without call steps | tst.js:31:9:31:23 | getConnection() |
+| type tracker without call steps | tst.js:40:7:40:21 | getConnection() |
+| type tracker without call steps | tst.js:48:7:48:21 | getConnection() |
+| type tracker without call steps | tst.js:54:37:54:51 | getConnection() |
+| type tracker without call steps | tst.js:57:14:57:48 | config. ... ction') |
 dataCallback
-| tst.js:9:11:9:12 | cb |
-| tst.js:20:1:22:1 | functio ... ata);\\n} |
-| tst.js:29:26:29:27 | cb |
-| tst.js:32:17:32:26 | data => {} |
-| tst.js:37:10:37:19 | data => {} |
-| tst.js:39:32:39:45 | getDataCurry() |
-| tst.js:44:19:44:20 | cb |
-| tst.js:47:32:47:60 | identit ... llback) |
+| tst.js:10:11:10:12 | cb |
+| tst.js:21:1:23:1 | functio ... ata);\\n} |
+| tst.js:30:26:30:27 | cb |
+| tst.js:33:17:33:26 | data => {} |
+| tst.js:38:10:38:19 | data => {} |
+| tst.js:40:32:40:45 | getDataCurry() |
+| tst.js:45:19:45:20 | cb |
+| tst.js:48:32:48:60 | identit ... llback) |
+| tst.js:58:16:58:22 | x => {} |
 dataValue
-| tst.js:20:18:20:21 | data |
-| tst.js:24:19:24:22 | data |
-| tst.js:32:17:32:20 | data |
-| tst.js:37:10:37:13 | data |
+| tst.js:21:18:21:21 | data |
+| tst.js:25:19:25:22 | data |
+| tst.js:33:17:33:20 | data |
+| tst.js:38:10:38:13 | data |
+| tst.js:58:16:58:16 | x |
diff --git a/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql b/javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql
@@ -1,4 +1,5 @@
 import javascript
+import CustomStep
 
 string chainableMethod() {
   result = "chain1" or
diff --git a/javascript/ql/test/library-tests/TypeTracking/tst.js b/javascript/ql/test/library-tests/TypeTracking/tst.js
@@ -1,4 +1,5 @@
 import myapi from "@test/myapi";
+import config from "@test/myconfig";
 
 let api = new myapi();
 
@@ -49,3 +50,10 @@ identity(fakeGetDataCallback);
 
 function realGetDataCallback(data) {}    // not found due to missing summarization
 function fakeGetDataCallback(notData) {} // should not be found
+
+config.setConfigValue('connection', getConnection());
+
+function getFromConfigFramework() {
+  let conn = config.getConfigValue('connection');
+  conn.getData(x => {});
+}

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`import javascript`
	`2`	`+import CustomStep`
`2`	`3`
`3`	`4`	`string chainableMethod() {`
`4`	`5`	`result = "chain1" or`