From c7295a09cd775e036c0d7e985a51b142c0586d09 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 7 Mar 2024 11:55:56 +0100 Subject: [PATCH 1/3] JS: Benign test output update --- .../CallGraphs/FullTest/tests.expected | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected b/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected index 0750c30359b..4d2c78206a4 100644 --- a/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected +++ b/javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected @@ -135,6 +135,7 @@ test_getAFunctionValue | tst.js:3:1:3:1 | h | tst.js:3:5:3:17 | function() {} | | tst.js:3:1:3:17 | h = function() {} | tst.js:3:5:3:17 | function() {} | | tst.js:3:5:3:17 | function() {} | tst.js:3:5:3:17 | function() {} | +| tst.js:4:1:4:1 | k | tst.js:2:9:2:21 | function() {} | | tst.js:4:1:4:5 | k = g | tst.js:2:9:2:21 | function() {} | | tst.js:4:5:4:5 | g | tst.js:2:9:2:21 | function() {} | | tst.js:6:1:6:1 | f | tst.js:1:1:1:15 | function f() {} | @@ -142,13 +143,23 @@ test_getAFunctionValue | tst.js:8:1:8:1 | h | tst.js:3:5:3:17 | function() {} | | tst.js:9:1:9:1 | k | tst.js:2:9:2:21 | function() {} | | tst.js:11:1:20:1 | functio ... \\tf();\\n} | tst.js:11:1:20:1 | functio ... \\tf();\\n} | +| tst.js:11:12:11:12 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:11:12:11:12 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:12:6:12:6 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:12:6:12:27 | n | tst.js:2:9:2:21 | function() {} | | tst.js:12:6:12:27 | n | tst.js:12:15:12:27 | function() {} | +| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} | +| tst.js:12:10:12:27 | m \|\| function() {} | tst.js:2:9:2:21 | function() {} | | tst.js:12:10:12:27 | m \|\| function() {} | tst.js:12:15:12:27 | function() {} | | tst.js:12:15:12:27 | function() {} | tst.js:12:15:12:27 | function() {} | | tst.js:13:2:13:16 | function p() {} | tst.js:13:2:13:16 | function p() {} | | tst.js:13:11:13:11 | p | tst.js:13:2:13:16 | function p() {} | +| tst.js:14:2:14:2 | m | tst.js:2:9:2:21 | function() {} | | tst.js:15:2:15:2 | l | tst.js:11:1:20:1 | functio ... \\tf();\\n} | | tst.js:16:2:16:17 | arguments.callee | tst.js:11:1:20:1 | functio ... \\tf();\\n} | +| tst.js:17:2:17:2 | n | tst.js:2:9:2:21 | function() {} | | tst.js:17:2:17:2 | n | tst.js:12:15:12:27 | function() {} | | tst.js:18:2:18:2 | p | tst.js:13:2:13:16 | function p() {} | | tst.js:19:2:19:2 | f | tst.js:1:1:1:15 | function f() {} | @@ -463,8 +474,10 @@ test_getACallee | tst.js:7:1:7:3 | g() | tst.js:2:9:2:21 | function() {} | | tst.js:8:1:8:3 | h() | tst.js:3:5:3:17 | function() {} | | tst.js:9:1:9:3 | k() | tst.js:2:9:2:21 | function() {} | +| tst.js:14:2:14:4 | m() | tst.js:2:9:2:21 | function() {} | | tst.js:15:2:15:4 | l() | tst.js:11:1:20:1 | functio ... \\tf();\\n} | | tst.js:16:2:16:19 | arguments.callee() | tst.js:11:1:20:1 | functio ... \\tf();\\n} | +| tst.js:17:2:17:4 | n() | tst.js:2:9:2:21 | function() {} | | tst.js:17:2:17:4 | n() | tst.js:12:15:12:27 | function() {} | | tst.js:18:2:18:4 | p() | tst.js:13:2:13:16 | function p() {} | | tst.js:19:2:19:4 | f() | tst.js:1:1:1:15 | function f() {} | From 6ebebc131e0b5b62739bb1a3f8f05c057f073367 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 7 Mar 2024 12:45:48 +0100 Subject: [PATCH 2/3] 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 91a0181cfb7564f4709bc575c8fcbc6775d7dc1c Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 7 Mar 2024 11:51:06 +0100 Subject: [PATCH 3/3] 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 682c9a2dffa..4420faacbfe 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 | () {} |