diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index f8b2118a55fb..0d05e1d81f4d 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -31,27 +31,16 @@ private class RemoteFlowSourceFromCsv extends RemoteFlowSource { override string getSourceType() { result = "Remote flow" } } -/** - * Like `ModelOutput::summaryStep` but with API nodes mapped to data-flow nodes. - */ -private predicate summaryStepNodes(DataFlow::Node pred, DataFlow::Node succ, string kind) { - exists(API::Node predNode, API::Node succNode | - Specific::summaryStep(predNode, succNode, kind) and - pred = predNode.getARhs() and - succ = succNode.getAnImmediateUse() - ) -} - /** 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") + Specific::summaryStep(pred, succ, "value") } } /** 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") + Specific::summaryStep(pred, succ, "taint") } } diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index 4681c2b91a54..e1eb2cfee8dd 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -132,6 +132,9 @@ module ModelInput { * indicates that for each call to `(package, type, path)`, the value referred to by `input` * can flow to the value referred to by `output`. * + * Alternatively, if `input` is the empty string and `output` consists of a single token, + * the value referred to by `(package, type, path)` may flow to the value referred to by `output`. + * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps, * respectively. */ diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index 752acea10c60..fed59af69399 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -197,15 +197,26 @@ private API::Node getNodeFromInputOutputPath(API::InvokeNode baseNode, AccessPat /** * Holds if a CSV summary contributed the step `pred -> succ` of the given `kind`. */ -predicate summaryStep(API::Node pred, API::Node succ, string kind) { +predicate summaryStep(DataFlow::Node pred, DataFlow::Node succ, string kind) { exists( string package, string type, string path, API::InvokeNode base, AccessPath input, AccessPath output | ModelOutput::relevantSummaryModel(package, type, path, input, output, kind) and ModelOutput::resolvedSummaryBase(package, type, path, base) and - pred = getNodeFromInputOutputPath(base, input) and - succ = getNodeFromInputOutputPath(base, output) + pred = getNodeFromInputOutputPath(base, input).getARhs() and + succ = getNodeFromInputOutputPath(base, output).getAnImmediateUse() + ) + or + exists( + string package, string type, string path, API::Node base, AccessPath output, + DataFlow::PropRead read + | + ModelOutput::relevantSummaryModel(package, type, path, "", output, kind) and + base = getNodeFromPath(package, type, path) and + read = getSuccessorFromNode(base, output).getAnImmediateUse() and + pred = read.getBase() and + succ = read ) } diff --git a/javascript/ql/test/library-tests/frameworks/data/test.expected b/javascript/ql/test/library-tests/frameworks/data/test.expected index e25a105b7598..e0fb3b1e8e52 100644 --- a/javascript/ql/test/library-tests/frameworks/data/test.expected +++ b/javascript/ql/test/library-tests/frameworks/data/test.expected @@ -55,6 +55,7 @@ taintFlow | test.js:182:12:182:19 | source() | test.js:182:12:182:19 | source() | | test.js:187:31:187:31 | x | test.js:189:10:189:10 | x | | test.js:203:32:203:39 | source() | test.js:203:32:203:39 | source() | +| test.js:209:13:209:20 | source() | test.js:210:8:210:19 | obj.self.foo | isSink | test.js:54:18:54:25 | source() | test-sink | | test.js:55:22:55:29 | source() | test-sink | diff --git a/javascript/ql/test/library-tests/frameworks/data/test.js b/javascript/ql/test/library-tests/frameworks/data/test.js index d31016e3fdad..a415ade61d4f 100644 --- a/javascript/ql/test/library-tests/frameworks/data/test.js +++ b/javascript/ql/test/library-tests/frameworks/data/test.js @@ -203,3 +203,9 @@ class OtherClass { this.accessorAroundField = source(); // NOT OK } } + +function testFlowThroughProp() { + const obj = new testlib.Obj(); + obj.foo = source(); + sink(obj.self.foo); // NOT OK +} diff --git a/javascript/ql/test/library-tests/frameworks/data/test.ql b/javascript/ql/test/library-tests/frameworks/data/test.ql index bdc08e3706fe..532192ce800e 100644 --- a/javascript/ql/test/library-tests/frameworks/data/test.ql +++ b/javascript/ql/test/library-tests/frameworks/data/test.ql @@ -14,6 +14,7 @@ class Steps extends ModelInput::SummaryModelCsv { "testlib;;Member[preserveAllButFirstArgument];Argument[1..];ReturnValue;taint", "testlib;;Member[preserveAllIfCall].Call;Argument[0..];ReturnValue;taint", "testlib;;Member[getSource].ReturnValue.Member[continue];Argument[this];ReturnValue;taint", + "testlib;;Member[Obj].Instance;;Member[self];value", ] } } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index 4681c2b91a54..e1eb2cfee8dd 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -132,6 +132,9 @@ module ModelInput { * indicates that for each call to `(package, type, path)`, the value referred to by `input` * can flow to the value referred to by `output`. * + * Alternatively, if `input` is the empty string and `output` consists of a single token, + * the value referred to by `(package, type, path)` may flow to the value referred to by `output`. + * * `kind` should be either `value` or `taint`, for value-preserving or taint-preserving steps, * respectively. */