From 7cd84c8f0ac0072348d100de75d2df2c9e35e372 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Feb 2024 09:59:44 +0100 Subject: [PATCH 1/5] JS: Add type-tracking test --- .../TypeTracking/TypeTracking.expected | 2 ++ .../library-tests/TypeTracking/summarize.js | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 javascript/ql/test/library-tests/TypeTracking/summarize.js diff --git a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected index e69de29bb2d1..75ca5993b09d 100644 --- a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected +++ b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected @@ -0,0 +1,2 @@ +| summarize.js:30:14:30:26 | // track: obj | Failed to track obj here. | +| summarize.js:33:14:33:26 | // track: obj | Failed to track obj here. | diff --git a/javascript/ql/test/library-tests/TypeTracking/summarize.js b/javascript/ql/test/library-tests/TypeTracking/summarize.js new file mode 100644 index 000000000000..e4aa9ebdb855 --- /dev/null +++ b/javascript/ql/test/library-tests/TypeTracking/summarize.js @@ -0,0 +1,33 @@ +import 'dummy'; + +function identity(x) { + return x; +} +function load(x) { + return x.loadProp; +} +function store(x) { + return { storeProp: x }; +} +function loadStore(x) { + return { storeProp: x.loadProp }; +} + +identity({}); +load({}); +store({}); +loadStore({}); + +const obj = {}; // name: obj + +let x = identity(obj); +x; // track: obj + +x = load({ loadProp: obj }); +x; // track: obj + +x = store(obj); +x.storeProp; // track: obj + +x = loadStore({ loadProp: obj }); +x.storeProp; // track: obj From 3ad83cc09812fe0106d39c12bc1dd3c617852e02 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 28 Feb 2024 22:02:07 +0100 Subject: [PATCH 2/5] JS: Summarise store steps for type tracking --- .../ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll | 3 +++ .../ql/test/library-tests/TypeTracking/TypeTracking.expected | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll index 6593df326155..c59b2aa58da9 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll @@ -156,6 +156,9 @@ private module Cached { exists(string prop | param.getAPropertyRead(prop).flowsTo(fun.getAReturn()) and summary = LoadStep(prop) + or + fun.getAReturn().getALocalSource().getAPropertySource(prop) = param and + summary = StoreStep(prop) ) ) and if param = fun.getAParameter() diff --git a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected index 75ca5993b09d..4670d4f9253d 100644 --- a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected +++ b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected @@ -1,2 +1 @@ -| summarize.js:30:14:30:26 | // track: obj | Failed to track obj here. | | summarize.js:33:14:33:26 | // track: obj | Failed to track obj here. | From f384afbaf6bc87d60de0c630756d4b7e722d19a3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Feb 2024 10:00:45 +0100 Subject: [PATCH 3/5] JS: Also summarize loadStore steps --- .../dataflow/internal/StepSummary.qll | 27 +++++++++++++++++++ .../TypeTracking/TypeTracking.expected | 1 - 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll index c59b2aa58da9..829e02591a08 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll @@ -45,6 +45,8 @@ private module Cached { CopyStep(PropertyName prop) or LoadStoreStep(PropertyName fromProp, PropertyName toProp) { SharedTypeTrackingStep::loadStoreStep(_, _, fromProp, toProp) + or + summarizedLoadStoreStep(_, _, fromProp, toProp) } or WithoutPropStep(PropertySet props) { SharedTypeTrackingStep::withoutPropStep(_, _, props) } } @@ -69,6 +71,26 @@ private module Cached { AccessPath::isAssignedInUniqueFile(global) } + bindingset[fun] + pragma[inline_late] + private DataFlow::PropRead getStoredPropRead(DataFlow::FunctionNode fun, string storeProp) { + result = fun.getAReturn().getALocalSource().getAPropertySource(storeProp) + } + + /** + * Holds if `loadProp` of `parameter` is stored in the `storeProp` property of the return value of `fun`. + */ + pragma[nomagic] + private predicate summarizedLoadStoreStep( + DataFlow::ParameterNode param, DataFlow::FunctionNode fun, string loadProp, string storeProp + ) { + exists(DataFlow::PropRead read | + read = getStoredPropRead(fun, storeProp) and + read.getBase().getALocalSource() = param and + read.getPropertyName() = loadProp + ) + } + /** * INTERNAL: Use `TypeBackTracker.smallstep()` instead. */ @@ -160,6 +182,11 @@ private module Cached { fun.getAReturn().getALocalSource().getAPropertySource(prop) = param and summary = StoreStep(prop) ) + or + exists(string loadProp, string storeProp | + summarizedLoadStoreStep(param, fun, loadProp, storeProp) and + summary = LoadStoreStep(loadProp, storeProp) + ) ) and if param = fun.getAParameter() then diff --git a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected index 4670d4f9253d..e69de29bb2d1 100644 --- a/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected +++ b/javascript/ql/test/library-tests/TypeTracking/TypeTracking.expected @@ -1 +0,0 @@ -| summarize.js:33:14:33:26 | // track: obj | Failed to track obj here. | From 13e3a5158eaa3f8c685ab342e2fba64cac10b296 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 29 Feb 2024 13:59:25 +0100 Subject: [PATCH 4/5] JS: Fix qldoc --- .../ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll index 829e02591a08..435d4d82ed57 100644 --- a/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll +++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/StepSummary.qll @@ -78,7 +78,7 @@ private module Cached { } /** - * Holds if `loadProp` of `parameter` is stored in the `storeProp` property of the return value of `fun`. + * Holds if `loadProp` of `param` is stored in the `storeProp` property of the return value of `fun`. */ pragma[nomagic] private predicate summarizedLoadStoreStep( From fc5b9e2796178bb9fd250a76eb827994d56e1187 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 8 Mar 2024 10:34:39 +0100 Subject: [PATCH 5/5] JS: Expand test case --- .../ql/test/library-tests/TypeTracking/summarize.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/javascript/ql/test/library-tests/TypeTracking/summarize.js b/javascript/ql/test/library-tests/TypeTracking/summarize.js index e4aa9ebdb855..08f09e80c7c5 100644 --- a/javascript/ql/test/library-tests/TypeTracking/summarize.js +++ b/javascript/ql/test/library-tests/TypeTracking/summarize.js @@ -12,11 +12,16 @@ function store(x) { function loadStore(x) { return { storeProp: x.loadProp }; } +function loadStore2(x) { + let mid = x.loadProp; + return { storeProp: mid }; +} identity({}); load({}); store({}); loadStore({}); +loadStore2({}); const obj = {}; // name: obj @@ -31,3 +36,6 @@ x.storeProp; // track: obj x = loadStore({ loadProp: obj }); x.storeProp; // track: obj + +x = loadStore2({ loadProp: obj }); +x.storeProp; // track: obj