Merge pull request #20169 from hvitved/javascript/legacy-summary-steps

JS: Generate legacy flow steps for all flow summaries

Author: hvitved

javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll

@@ -21,6 +21,8 @@ private import internal.ApiGraphModels as Shared
 private import internal.ApiGraphModelsSpecific as Specific
 private import semmle.javascript.dataflow.internal.FlowSummaryPrivate
 private import semmle.javascript.endpoints.EndpointNaming as EndpointNaming
+private import semmle.javascript.dataflow.AdditionalFlowSteps
+private import semmle.javascript.dataflow.AdditionalTaintSteps
 import Shared::ModelInput as ModelInput
 import Shared::ModelOutput as ModelOutput
 
@@ -87,9 +89,6 @@ private predicate shouldInduceStepsFromSummary(string type, string path) {
 pragma[nomagic]
 private predicate relevantInputOutputPath(API::InvokeNode base, AccessPath inputOrOutput) {
   exists(string type, string input, string output, string path |
-    // If the summary for 'callable' could not be handled as a flow summary, we need to evaluate
-    // its inputs and outputs to a set of nodes, so we can generate steps instead.
-    shouldInduceStepsFromSummary(type, path) and
     ModelOutput::resolvedSummaryBase(type, path, base) and
     ModelOutput::relevantSummaryModel(type, path, input, output, _, _) and
     inputOrOutput = [input, output]
@@ -118,22 +117,26 @@ private API::Node getNodeFromInputOutputPath(API::InvokeNode baseNode, AccessPat
   result = getNodeFromInputOutputPath(baseNode, path, path.getNumToken())
 }
 
-private predicate summaryStep(API::Node pred, API::Node succ, string kind) {
+private predicate summaryStep(API::Node pred, API::Node succ, string kind, boolean shouldInduceSteps) {
   exists(string type, string path, API::InvokeNode base, AccessPath input, AccessPath output |
-    shouldInduceStepsFromSummary(type, path) and
     ModelOutput::relevantSummaryModel(type, path, input, output, kind, _) and
     ModelOutput::resolvedSummaryBase(type, path, base) and
     pred = getNodeFromInputOutputPath(base, input) and
-    succ = getNodeFromInputOutputPath(base, output)
+    succ = getNodeFromInputOutputPath(base, output) and
+    if shouldInduceStepsFromSummary(type, path)
+    then shouldInduceSteps = true
+    else shouldInduceSteps = false
   )
 }
 
 /**
  * Like `ModelOutput::summaryStep` but with API nodes mapped to data-flow nodes.
  */
-private predicate summaryStepNodes(DataFlow::Node pred, DataFlow::Node succ, string kind) {
+private predicate summaryStepNodes(
+  DataFlow::Node pred, DataFlow::Node succ, string kind, boolean shouldInduceSteps
+) {
   exists(API::Node predNode, API::Node succNode |
-    summaryStep(predNode, succNode, kind) and
+    summaryStep(predNode, succNode, kind, shouldInduceSteps) and
     pred = predNode.asSink() and
     succ = succNode.asSource()
   )
@@ -142,14 +145,26 @@ private predicate summaryStepNodes(DataFlow::Node pred, DataFlow::Node succ, str
 /** Data flow steps induced by summary models of kind `value`. */
 private class DataFlowStepFromSummary extends DataFlow::SharedFlowStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    summaryStepNodes(pred, succ, "value")
+    summaryStepNodes(pred, succ, "value", true)
+  }
+}
+
+private class LegacyDataFlowStepFromSummary extends LegacyFlowStep {
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    summaryStepNodes(pred, succ, "value", false)
   }
 }
 
 /** Taint steps induced by summary models of kind `taint`. */
 private class TaintStepFromSummary extends TaintTracking::SharedTaintStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    summaryStepNodes(pred, succ, "taint")
+    summaryStepNodes(pred, succ, "taint", true)
+  }
+}
+
+private class LegacyTaintStepFromSummary extends LegacyTaintStep {
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    summaryStepNodes(pred, succ, "taint", false)
   }
 }
 

javascript/ql/test/library-tests/frameworks/data/test.expected

@@ -1,21 +1,4 @@
 legacyDataFlowDifference
-| test.js:5:30:5:37 | source() | test.js:5:8:5:38 | testlib ... urce()) | only flow with NEW data flow library |
-| test.js:6:22:6:29 | source() | test.js:6:8:6:30 | preserv ... urce()) | only flow with NEW data flow library |
-| test.js:7:41:7:48 | source() | test.js:7:8:7:49 | require ... urce()) | only flow with NEW data flow library |
-| test.js:11:38:11:45 | source() | test.js:11:8:11:55 | testlib ... , 1, 1) | only flow with NEW data flow library |
-| test.js:13:44:13:51 | source() | test.js:13:8:13:55 | testlib ... e(), 1) | only flow with NEW data flow library |
-| test.js:17:47:17:54 | source() | test.js:17:8:17:61 | testlib ... , 1, 1) | only flow with NEW data flow library |
-| test.js:18:50:18:57 | source() | test.js:18:8:18:61 | testlib ... e(), 1) | only flow with NEW data flow library |
-| test.js:19:53:19:60 | source() | test.js:19:8:19:61 | testlib ... urce()) | only flow with NEW data flow library |
-| test.js:21:34:21:41 | source() | test.js:21:8:21:51 | testlib ... , 1, 1) | only flow with NEW data flow library |
-| test.js:22:37:22:44 | source() | test.js:22:8:22:51 | testlib ... , 1, 1) | only flow with NEW data flow library |
-| test.js:23:40:23:47 | source() | test.js:23:8:23:51 | testlib ... e(), 1) | only flow with NEW data flow library |
-| test.js:24:43:24:50 | source() | test.js:24:8:24:51 | testlib ... urce()) | only flow with NEW data flow library |
-| test.js:31:29:31:36 | source() | test.js:32:10:32:10 | y | only flow with NEW data flow library |
-| test.js:37:29:37:36 | source() | test.js:38:10:38:10 | y | only flow with NEW data flow library |
-| test.js:43:29:43:36 | source() | test.js:44:10:44:10 | y | only flow with NEW data flow library |
-| test.js:47:33:47:40 | source() | test.js:49:10:49:13 | this | only flow with NEW data flow library |
-| test.js:102:16:102:34 | testlib.getSource() | test.js:104:8:104:24 | source.continue() | only flow with NEW data flow library |
 consistencyIssue
 taintFlow
 | guardedRouteHandler.js:6:10:6:28 | req.injectedReqData | guardedRouteHandler.js:6:10:6:28 | req.injectedReqData |