From 84c788a027b1eb70394d707640fc7e771d8d6dd8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Oct 2025 11:31:08 +0200 Subject: [PATCH 1/3] JS: Add API graph test for explicit 'this' passing --- .../test/ApiGraphs/explicit-this/VerifyAssertions.expected | 0 .../ql/test/ApiGraphs/explicit-this/VerifyAssertions.ql | 1 + javascript/ql/test/ApiGraphs/explicit-this/package.json | 6 ++++++ javascript/ql/test/ApiGraphs/explicit-this/tst.js | 7 +++++++ 4 files changed, 14 insertions(+) create mode 100644 javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected create mode 100644 javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.ql create mode 100644 javascript/ql/test/ApiGraphs/explicit-this/package.json create mode 100644 javascript/ql/test/ApiGraphs/explicit-this/tst.js diff --git a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.ql b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.ql new file mode 100644 index 00000000000..b9c54e26072 --- /dev/null +++ b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.ql @@ -0,0 +1 @@ +import ApiGraphs.VerifyAssertions diff --git a/javascript/ql/test/ApiGraphs/explicit-this/package.json b/javascript/ql/test/ApiGraphs/explicit-this/package.json new file mode 100644 index 00000000000..f48acd62360 --- /dev/null +++ b/javascript/ql/test/ApiGraphs/explicit-this/package.json @@ -0,0 +1,6 @@ +{ + "name": "explicit-this", + "dependencies": { + "something": "*" + } +} diff --git a/javascript/ql/test/ApiGraphs/explicit-this/tst.js b/javascript/ql/test/ApiGraphs/explicit-this/tst.js new file mode 100644 index 00000000000..a3f5ecff21e --- /dev/null +++ b/javascript/ql/test/ApiGraphs/explicit-this/tst.js @@ -0,0 +1,7 @@ +const lib = require('something'); + +function f() { + this.two(); /** use=moduleImport("something").getMember("exports").getMember("one").getMember("two").getReturn() */ +} + +f.call(lib.one); From 4d3319024174f7cae61b348ea271024b6f775c94 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Oct 2025 11:22:39 +0200 Subject: [PATCH 2/3] JS: Restrict this-argument passing in API graphs --- javascript/ql/lib/semmle/javascript/ApiGraphs.qll | 8 ++++++-- .../ApiGraphs/explicit-this/VerifyAssertions.expected | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 850e9224451..647d8f3b837 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1324,7 +1324,9 @@ module API { exists(DataFlow::TypeTracker t, StepSummary summary, DataFlow::SourceNode prev | prev = trackUseNode(nd, promisified, boundArgs, prop, t) and StepSummary::step(prev, res, summary) and - result = t.append(summary) + result = t.append(summary) and + // Block argument-passing into 'this' + not (summary = CallStep() and res instanceof DataFlow::ThisNode) ) } @@ -1381,7 +1383,9 @@ module API { exists(DataFlow::TypeBackTracker t, StepSummary summary, DataFlow::Node next | next = trackDefNode(nd, t) and StepSummary::step(prev, next, summary) and - result = t.prepend(summary) + result = t.prepend(summary) and + // Block argument-passing steps from 'this' back to a receiver + not (summary = CallStep() and prev instanceof DataFlow::ThisNode) ) } diff --git a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected index e69de29bb2d..daff5d3e13b 100644 --- a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected +++ b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected @@ -0,0 +1 @@ +| tst.js:4:17:4:119 | /** use ... rn() */ | use moduleImport("something").getMember("exports").getMember("one") has no outgoing edge labelled getMember("two"); it has no outgoing edges at all. | From 587ad5c60041b4ed425e48cf77fa32bf0ab85953 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 6 Oct 2025 11:39:54 +0200 Subject: [PATCH 3/3] JS: Refine criteria so that explicit this-passing is not affected --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 8 ++++---- .../javascript/dataflow/TypeTracking.qll | 4 ++++ .../dataflow/internal/StepSummary.qll | 18 +++++++++++++++++- .../explicit-this/VerifyAssertions.expected | 1 - 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 647d8f3b837..1a96e25b3b9 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -1325,8 +1325,8 @@ module API { prev = trackUseNode(nd, promisified, boundArgs, prop, t) and StepSummary::step(prev, res, summary) and result = t.append(summary) and - // Block argument-passing into 'this' - not (summary = CallStep() and res instanceof DataFlow::ThisNode) + // Block argument-passing into 'this' when it determines the call target + not summary = CallReceiverStep() ) } @@ -1384,8 +1384,8 @@ module API { next = trackDefNode(nd, t) and StepSummary::step(prev, next, summary) and result = t.prepend(summary) and - // Block argument-passing steps from 'this' back to a receiver - not (summary = CallStep() and prev instanceof DataFlow::ThisNode) + // Block argument-passing steps from 'this' back to a receiver when it determines the call target + not summary = CallReceiverStep() ) } diff --git a/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll b/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll index 9e912a336f6..e4c8f162972 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll @@ -65,6 +65,8 @@ class TypeTracker extends TTypeTracker { or step = CallStep() and result = MkTypeTracker(true, prop) or + step = CallReceiverStep() and result = MkTypeTracker(true, prop) + or step = ReturnStep() and hasCall = false and result = this or step = LoadStep(prop) and result = MkTypeTracker(hasCall, "") @@ -238,6 +240,8 @@ class TypeBackTracker extends TTypeBackTracker { or step = CallStep() and hasReturn = false and result = this or + step = CallReceiverStep() and hasReturn = false and result = this + or step = ReturnStep() and result = MkTypeBackTracker(true, prop) or exists(string p | step = LoadStep(p) and prop = "" and result = MkTypeBackTracker(hasReturn, p)) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll index 2bcd89130a9..fed492074b6 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll @@ -43,6 +43,7 @@ private module Cached { newtype TStepSummary = LevelStep() or CallStep() or + CallReceiverStep() or ReturnStep() or StoreStep(PropertyName prop) or LoadStep(PropertyName prop) or @@ -101,6 +102,15 @@ private module Cached { ) } + pragma[nomagic] + private predicate isReceiverForMethodDispatch(DataFlow::Node node) { + exists(DataFlow::SourceNode base, DataFlow::CallNode invoke | + node = invoke.getReceiver() and + base = node.getALocalSource() and + invoke.getCalleeNode() = base.getAPropertyRead() + ) + } + /** * INTERNAL: Use `TypeBackTracker.smallstep()` instead. */ @@ -116,7 +126,11 @@ private module Cached { or // Flow into function callStep(pred, succ) and - summary = CallStep() + ( + if isReceiverForMethodDispatch(pred) + then summary = CallReceiverStep() + else summary = CallStep() + ) or // Flow out of function returnStep(pred, succ) and @@ -251,6 +265,8 @@ class StepSummary extends TStepSummary { or this instanceof CallStep and result = "call" or + this instanceof CallReceiverStep and result = "call-receiver" + or this instanceof ReturnStep and result = "return" or exists(string prop | this = StoreStep(prop) | result = "store " + prop) diff --git a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected index daff5d3e13b..e69de29bb2d 100644 --- a/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected +++ b/javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected @@ -1 +0,0 @@ -| tst.js:4:17:4:119 | /** use ... rn() */ | use moduleImport("something").getMember("exports").getMember("one") has no outgoing edge labelled getMember("two"); it has no outgoing edges at all. |