From f2939bd05be0bccc5fe8038b53e0ac022f206121 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 7 Mar 2024 12:45:48 +0100 Subject: [PATCH 1/2] JS: Add test case --- .../CallGraphs/AnnotatedTest/Test.expected | 1 + .../AnnotatedTest/implied-receiver.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/implied-receiver.js diff --git a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected index 8182d017414..9d59da5ccad 100644 --- a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected +++ b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected @@ -2,6 +2,7 @@ spuriousCallee missingCallee | constructor-field.ts:40:5:40:14 | f3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls | | constructor-field.ts:71:1:71:11 | bf3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls | +| implied-receiver.js:7:13:7:25 | this.member() | implied-receiver.js:17:22:19:1 | functio ... n 42;\\n} | -1 | calls | badAnnotation accessorCall | accessors.js:12:1:12:5 | obj.f | accessors.js:5:8:5:12 | () {} | diff --git a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/implied-receiver.js b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/implied-receiver.js new file mode 100644 index 00000000000..22638f35b56 --- /dev/null +++ b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/implied-receiver.js @@ -0,0 +1,19 @@ +import 'dummy'; + +function fooFactoryFactory() { + return function fooFactory() { + return function foo() { + /** calls:F.member */ + this.member(); + } + } +} + +function F() { + this.foo = fooFactoryFactory()(); +} + +/** name:F.member */ +F.prototype.member = function() { + return 42; +}; From 22b56a4a40761e6168cc4ac04610d2dd69fffe6f Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 7 Mar 2024 11:51:06 +0100 Subject: [PATCH 2/2] JS: More implied receiver steps --- .../dataflow/internal/CallGraphs.qll | 23 +++++++++++++++++++ .../CallGraphs/AnnotatedTest/Test.expected | 1 - 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll index 7e55944038b..541e3a6f3e9 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll @@ -279,6 +279,20 @@ module CallGraph { StepSummary::step(getAnAllocationSiteRef(node), result, objectWithMethodsStep()) } + /** + * Holds if `function` flows to a property of `host` via non-local data flow. + */ + pragma[nomagic] + private predicate complexMethodInstallation( + DataFlow::SourceNode host, DataFlow::FunctionNode function + ) { + not function = getAMethodOnObject(_) and + exists(DataFlow::TypeTracker t | + getAFunctionReference(function, 0, t) = host.getAPropertySource() and + t.start() // require call bit to be false + ) + } + /** * Holds if `pred` is assumed to flow to `succ` because a method is stored on an object that is assumed * to be the receiver of calls to that method. @@ -291,9 +305,18 @@ module CallGraph { */ cached predicate impliedReceiverStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) { + // To avoid double-recursion, we handle either complex flow for the host object, or for the function, but not both. exists(DataFlow::SourceNode host | + // Complex flow for the host object pred = getAnAllocationSiteRef(host) and succ = getAMethodOnObject(host).getReceiver() + or + // Complex flow for the function + exists(DataFlow::FunctionNode function | + complexMethodInstallation(host, function) and + pred = host and + succ = function.getReceiver() + ) ) } } diff --git a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected index 9d59da5ccad..8182d017414 100644 --- a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected +++ b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected @@ -2,7 +2,6 @@ spuriousCallee missingCallee | constructor-field.ts:40:5:40:14 | f3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls | | constructor-field.ts:71:1:71:11 | bf3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls | -| implied-receiver.js:7:13:7:25 | this.member() | implied-receiver.js:17:22:19:1 | functio ... n 42;\\n} | -1 | calls | badAnnotation accessorCall | accessors.js:12:1:12:5 | obj.f | accessors.js:5:8:5:12 | () {} |