From eb3c054b0fc8cd54424c430183f3adea569c9cc2 Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Tue, 5 Aug 2025 10:55:36 +0200 Subject: [PATCH] JS: Generate legacy flow steps for all flow summaries --- .../frameworks/data/ModelsAsData.qll | 35 +++++++++++++------ .../frameworks/data/test.expected | 17 --------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index 0e19e84b666f..82deb735c629 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/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) } } diff --git a/javascript/ql/test/library-tests/frameworks/data/test.expected b/javascript/ql/test/library-tests/frameworks/data/test.expected index 875b0189d619..0bc1b6b6ee07 100644 --- a/javascript/ql/test/library-tests/frameworks/data/test.expected +++ b/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 |