From 993345fb7bc73ecd10a2e2d22c4ec7d507064bbf Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 11 Mar 2019 17:13:41 +0000 Subject: [PATCH 1/2] JavaScript: Track Electron browser objects locally only. --- .../ql/src/semmle/javascript/frameworks/Electron.qll | 8 ++++---- .../frameworks/Electron/BrowserObject.expected | 4 ---- .../ql/test/library-tests/frameworks/Electron/IpcFlow.ql | 8 ++------ .../frameworks/Electron/WebContents.expected | 2 -- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll index d307084c10fb..744648469fe7 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Electron.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Electron.qll @@ -16,7 +16,7 @@ module Electron { /** * An instantiation of `BrowserWindow` or `BrowserView`. */ - abstract private class NewBrowserObject extends BrowserObject, DataFlow::TrackedNode { + abstract private class NewBrowserObject extends BrowserObject, DataFlow::SourceNode { DataFlow::NewNode self; NewBrowserObject() { this = self } @@ -250,10 +250,10 @@ module Electron { /** * An additional flow step via an Electron IPC message. */ - private class IPCAdditionalFlowStep extends DataFlow::Configuration { - IPCAdditionalFlowStep() { this instanceof DataFlow::Configuration } + private class IPCAdditionalFlowStep extends DataFlow::AdditionalFlowStep { + IPCAdditionalFlowStep() { ipcFlowStep(this, _) } - override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { ipcFlowStep(pred, succ) } } diff --git a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected index 579c5817840a..76865b0c1dba 100644 --- a/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected +++ b/javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected @@ -4,11 +4,7 @@ | electron.js:3:10:3:48 | new Bro ... s: {}}) | | electron.js:4:5:4:46 | bv | | electron.js:4:10:4:46 | new Bro ... s: {}}) | -| electron.js:35:14:35:14 | x | -| electron.js:36:12:36:12 | x | -| electron.js:39:1:39:7 | foo(bw) | | electron.js:39:5:39:6 | bw | -| electron.js:40:1:40:7 | foo(bv) | | electron.js:40:5:40:6 | bv | | electron.ts:3:12:3:13 | bw | | electron.ts:3:40:3:41 | bv | diff --git a/javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.ql b/javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.ql index 893b4fc75761..629f3c030fe9 100644 --- a/javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.ql +++ b/javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.ql @@ -1,9 +1,5 @@ import javascript -class TestConfig extends DataFlow::Configuration { - TestConfig() { this = "TestConfig" } -} - -from TestConfig cfg, DataFlow::Node pred, DataFlow::Node succ -where cfg.isAdditionalFlowStep(pred, succ) +from DataFlow::AdditionalFlowStep afs, DataFlow::Node pred, DataFlow::Node succ +where afs.step(pred, succ) select pred, succ diff --git a/javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected b/javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected index 4ae6c7e5d950..a55d4a16e4de 100644 --- a/javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected +++ b/javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected @@ -1,4 +1,2 @@ -| electron.js:39:1:39:19 | foo(bw).webContents | -| electron.js:40:1:40:19 | foo(bv).webContents | | electron.ts:4:3:4:16 | bw.webContents | | electron.ts:5:3:5:16 | bv.webContents | From 8e52528219ded21719529208059e2a019d4a5526 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 11 Mar 2019 17:13:52 +0000 Subject: [PATCH 2/2] JavaScript: Refactor `reachableFromInput` to improve join. --- .../javascript/dataflow/Configuration.qll | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll index 592e1a7f2e85..1e0dae0d7cf2 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/Configuration.qll @@ -570,10 +570,24 @@ private predicate reachableFromInput( callInputStep(f, invk, input, nd, cfg) and summary = PathSummary::level() or - exists(DataFlow::Node mid, PathSummary oldSummary, PathSummary newSummary | + exists(DataFlow::Node mid, PathSummary oldSummary | reachableFromInput(f, invk, input, mid, cfg, oldSummary) and - flowStep(mid, cfg, nd, newSummary) and - summary = oldSummary.append(newSummary) + appendStep(mid, cfg, oldSummary, nd, summary) + ) +} + +/** + * Holds if there is a step from `pred` to `succ` under `cfg` that can be appended + * to a path represented by `oldSummary` yielding a path represented by `newSummary`. + */ +pragma[noinline] +private predicate appendStep( + DataFlow::Node pred, DataFlow::Configuration cfg, PathSummary oldSummary, DataFlow::Node succ, + PathSummary newSummary +) { + exists(PathSummary stepSummary | + flowStep(pred, cfg, succ, stepSummary) and + newSummary = oldSummary.append(stepSummary) ) }