Skip to content

Commit 58cc8d0

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #936 from xiemaisi/js/revive-electron-support
Approved by esben-semmle
2 parents 46a1c75 + 0635e1b commit 58cc8d0

File tree

13 files changed

+335
-21
lines changed

13 files changed

+335
-21
lines changed

‎change-notes/1.20/analysis-javascript.md‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@
22

33
## General improvements
44

5-
* Support for popular libraries has been improved. Consequently, queries may produce better results on code bases that use the following features:
6-
- client-side code, for example [React](https://reactjs.org/) and [Vue](https://vuejs.org/)
7-
- cookies and webstorage, for example [js-cookie](https://github.com/js-cookie/js-cookie)
8-
- server-side code, for example [hapi](https://hapijs.com/)
9-
- asynchronous code, for example [a-sync-waterfall](https://www.npmjs.com/package/a-sync-waterfall)
5+
* Support for many frameworks and libraries has been improved, in particular including the following:
6+
- [a-sync-waterfall](https://www.npmjs.com/package/a-sync-waterfall)
7+
- [Electron](https://electronjs.org)
8+
- [hapi](https://hapijs.com/)
9+
- [js-cookie](https://github.com/js-cookie/js-cookie)
10+
- [React](https://reactjs.org/)
11+
- [Vue](https://vuejs.org/)
12+
1013
* File classification has been improved to recognize additional generated files, for example files from [HTML Tidy](html-tidy.org).
1114

1215
* The taint tracking library now recognizes flow through persistent storage, class fields, and callbacks in certain cases. Handling of regular expressions has also been improved. This may give more results for the security queries.

‎javascript/ql/src/semmle/javascript/frameworks/Electron.qll‎

Lines changed: 239 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,263 @@ import javascript
22

33
module Electron {
44
/**
5-
* A data flow node that is an Electron `webPreferences` property.
5+
* A `webPreferences` object.
66
*/
77
class WebPreferences extends DataFlow::ObjectLiteralNode {
8-
WebPreferences() {
9-
exists(BrowserObject bo | this = bo.getOptionArgument(0, "webPreferences").getALocalSource())
10-
}
8+
WebPreferences() { this = any(NewBrowserObject nbo).getWebPreferences() }
119
}
1210

1311
/**
14-
* A data flow node that creates a new `BrowserWindow` or `BrowserView`.
12+
* A data flow node that may contain a `BrowserWindow` or `BrowserView` object.
1513
*/
16-
abstract private class BrowserObject extends DataFlow::NewNode { }
14+
abstract class BrowserObject extends DataFlow::Node { }
15+
16+
/**
17+
* An instantiation of `BrowserWindow` or `BrowserView`.
18+
*/
19+
abstract private class NewBrowserObject extends BrowserObject, DataFlow::TrackedNode {
20+
DataFlow::NewNode self;
21+
22+
NewBrowserObject() { this = self }
23+
24+
/**
25+
* Gets the data flow node from which this instantiation takes its `webPreferences` object.
26+
*/
27+
DataFlow::SourceNode getWebPreferences() {
28+
result = self.getOptionArgument(0, "webPreferences").getALocalSource()
29+
}
30+
}
1731

1832
/**
19-
* A data flow node that creates a new `BrowserWindow`.
33+
* An instantiation of `BrowserWindow`.
2034
*/
21-
class BrowserWindow extends BrowserObject {
35+
class BrowserWindow extends NewBrowserObject {
2236
BrowserWindow() {
2337
this = DataFlow::moduleMember("electron", "BrowserWindow").getAnInstantiation()
2438
}
2539
}
2640

2741
/**
28-
* A data flow node that creates a new `BrowserView`.
42+
* An instantiation of `BrowserView`.
2943
*/
30-
class BrowserView extends BrowserObject {
44+
class BrowserView extends NewBrowserObject {
3145
BrowserView() { this = DataFlow::moduleMember("electron", "BrowserView").getAnInstantiation() }
3246
}
3347

48+
/**
49+
* An expression of type `BrowserWindow` or `BrowserView`.
50+
*/
51+
private class BrowserObjectByType extends BrowserObject {
52+
BrowserObjectByType() {
53+
exists(string tp | tp = "BrowserWindow" or tp = "BrowserView" |
54+
asExpr().getType().hasUnderlyingType("electron", tp)
55+
)
56+
}
57+
}
58+
59+
/**
60+
* A data flow node whose value may originate from a browser object instantiation.
61+
*/
62+
private class BrowserObjectByFlow extends BrowserObject {
63+
BrowserObjectByFlow() { any(NewBrowserObject nbo).flowsTo(this) }
64+
}
65+
66+
/**
67+
* A reference to the `webContents` property of a browser object.
68+
*/
69+
class WebContents extends DataFlow::SourceNode {
70+
WebContents() { this.(DataFlow::PropRead).accesses(any(BrowserObject bo), "webContents") }
71+
}
72+
73+
/**
74+
* Provides classes and predicates for modelling Electron inter-process communication (IPC).
75+
*/
76+
private module IPC {
77+
class Process extends string {
78+
Process() { this = "main" or this = "renderer" }
79+
80+
DataFlow::SourceNode getAnImport() {
81+
this = Process::main() and result = DataFlow::moduleMember("electron", "ipcMain")
82+
or
83+
this = Process::renderer() and result = DataFlow::moduleMember("electron", "ipcRenderer")
84+
}
85+
}
86+
87+
module Process {
88+
Process main() { result = "main" }
89+
90+
Process renderer() { result = "renderer" }
91+
}
92+
93+
/**
94+
* An IPC callback.
95+
*/
96+
class Callback extends DataFlow::FunctionNode {
97+
DataFlow::Node channel;
98+
99+
Process process;
100+
101+
Callback() {
102+
exists(DataFlow::MethodCallNode mc |
103+
mc = process.getAnImport().getAMemberCall("on") and
104+
this = mc.getCallback(1) and
105+
channel = mc.getArgument(0)
106+
)
107+
}
108+
109+
/** Gets the process on which this callback is executed. */
110+
Process getProcess() { result = process }
111+
112+
/** Gets the name of the channel the callback is listening on. */
113+
string getChannelName() { result = channel.asExpr().getStringValue() }
114+
115+
/** Gets the data flow node containing the message received by the callback. */
116+
DataFlow::Node getMessage() {
117+
result = getParameter(1)
118+
}
119+
}
120+
121+
/**
122+
* An IPC message.
123+
*/
124+
abstract class Message extends DataFlow::Node {
125+
/** Gets the process that sends this message. */
126+
abstract Process getProcess();
127+
128+
/** Gets the name of the channel this message is sent on. */
129+
abstract string getChannelName();
130+
}
131+
132+
/**
133+
* An IPC message sent directly from a process.
134+
*/
135+
class DirectMessage extends Message {
136+
DataFlow::MethodCallNode mc;
137+
138+
Process process;
139+
140+
DataFlow::Node channel;
141+
142+
boolean isSync;
143+
144+
DirectMessage() {
145+
exists(string send |
146+
send = "send" and isSync = false
147+
or
148+
send = "sendSync" and isSync = true
149+
|
150+
mc = process.getAnImport().getAMemberCall(send) and
151+
this = mc.getArgument(1) and
152+
channel = mc.getArgument(0)
153+
)
154+
}
155+
156+
override Process getProcess() { result = process }
157+
158+
override string getChannelName() { result = channel.asExpr().getStringValue() }
159+
}
160+
161+
/**
162+
* A synchronous IPC message sent directly from a process.
163+
*/
164+
class SyncDirectMessage extends DirectMessage {
165+
SyncDirectMessage() { isSync = true }
166+
167+
/** Gets the data flow node holding the reply to the message. */
168+
DataFlow::Node getReply() {
169+
result = mc
170+
}
171+
}
172+
173+
/**
174+
* An asynchronous IPC reply sent from within an IPC callback.
175+
*/
176+
class AsyncReplyMessage extends Message {
177+
Callback callback;
178+
179+
DataFlow::Node channel;
180+
181+
AsyncReplyMessage() {
182+
exists(DataFlow::MethodCallNode mc |
183+
mc = callback.getParameter(0).getAPropertyRead("sender").getAMemberCall("send") and
184+
this = mc.getArgument(1) and
185+
channel = mc.getArgument(0)
186+
)
187+
}
188+
189+
override Process getProcess() { result = callback.getProcess() }
190+
191+
override string getChannelName() { result = channel.asExpr().getStringValue() }
192+
}
193+
194+
/**
195+
* A synchronous IPC reply sent from within an IPC callback.
196+
*/
197+
class SyncReplyMessage extends Message {
198+
Callback callback;
199+
200+
SyncReplyMessage() {
201+
this = callback.getParameter(0).getAPropertyWrite("returnValue").getRhs()
202+
}
203+
204+
override Process getProcess() { result = callback.getProcess() }
205+
206+
override string getChannelName() { result = callback.getChannelName() }
207+
}
208+
209+
/**
210+
* An asynchronous Electron IPC message sent from the main process via a `webContents` object.
211+
*/
212+
class WebContentsMessage extends Message {
213+
DataFlow::Node channel;
214+
215+
WebContentsMessage() {
216+
exists(WebContents wc, DataFlow::MethodCallNode mc |
217+
wc.flowsTo(mc.getReceiver()) and
218+
this = mc.getArgument(1) and
219+
channel = mc.getArgument(0) and
220+
mc.getCalleeName() = "send"
221+
)
222+
}
223+
224+
override Process getProcess() { result = Process::main() }
225+
226+
override string getChannelName() { result = channel.asExpr().getStringValue() }
227+
}
228+
229+
/**
230+
* Holds if `pred` flows to `succ` via Electron IPC.
231+
*/
232+
private predicate ipcFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
233+
// match a message sent from one process with a callback parameter in the other process
234+
exists(Callback callback, Message msg |
235+
callback.getChannelName() = msg.getChannelName() and
236+
callback.getProcess() != msg.getProcess() and
237+
pred = msg and
238+
succ = callback.getMessage()
239+
)
240+
or
241+
// match a synchronous reply sent from one process with a `sendSync` call in the other process
242+
exists(SyncDirectMessage sendSync, SyncReplyMessage msg |
243+
sendSync.getChannelName() = msg.getChannelName() and
244+
sendSync.getProcess() != msg.getProcess() and
245+
pred = msg and
246+
succ = sendSync.getReply()
247+
)
248+
}
249+
250+
/**
251+
* An additional flow step via an Electron IPC message.
252+
*/
253+
private class IPCAdditionalFlowStep extends DataFlow::Configuration {
254+
IPCAdditionalFlowStep() { this instanceof DataFlow::Configuration }
255+
256+
override predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
257+
ipcFlowStep(pred, succ)
258+
}
259+
}
260+
}
261+
34262
/**
35263
* A Node.js-style HTTP or HTTPS request made using an Electron module.
36264
*/
@@ -68,7 +296,7 @@ module Electron {
68296

69297
override DataFlow::Node getADataNode() {
70298
exists(string name | name = "write" or name = "end" |
71-
result = this.(DataFlow::SourceNode).getAMethodCall(name).getArgument(0)
299+
result = this.getAMethodCall(name).getArgument(0)
72300
)
73301
}
74302
}

‎javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.expected‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
| electron.d.ts:2:16:2:28 | BrowserWindow |
2+
| electron.d.ts:3:16:3:26 | BrowserView |
3+
| electron.js:3:5:3:48 | bw |
4+
| electron.js:3:10:3:48 | new Bro ... s: {}}) |
5+
| electron.js:4:5:4:46 | bv |
6+
| electron.js:4:10:4:46 | new Bro ... s: {}}) |
7+
| electron.js:35:14:35:14 | x |
8+
| electron.js:36:12:36:12 | x |
9+
| electron.js:39:1:39:7 | foo(bw) |
10+
| electron.js:39:5:39:6 | bw |
11+
| electron.js:40:1:40:7 | foo(bv) |
12+
| electron.js:40:5:40:6 | bv |
13+
| electron.ts:3:12:3:13 | bw |
14+
| electron.ts:3:40:3:41 | bv |
15+
| electron.ts:4:3:4:4 | bw |
16+
| electron.ts:5:3:5:4 | bv |

‎javascript/ql/test/library-tests/frameworks/Electron/BrowserObject.ql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import javascript
2+
3+
from Electron::BrowserObject obj
4+
select obj

‎javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| electron.js:43:30:43:35 | 'pong' | electron.js:52:33:52:35 | arg |
2+
| electron.js:48:23:48:28 | 'pong' | electron.js:58:1:58:36 | ipcRend ... 'ping') |
3+
| electron.js:56:27:56:32 | 'ping' | electron.js:42:29:42:31 | arg |
4+
| electron.js:58:30:58:35 | 'ping' | electron.js:47:28:47:30 | arg |

‎javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.ql‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import javascript
2+
3+
class TestConfig extends DataFlow::Configuration {
4+
TestConfig() { this = "TestConfig" }
5+
}
6+
7+
from TestConfig cfg, DataFlow::Node pred, DataFlow::Node succ
8+
where cfg.isAdditionalFlowStep(pred, succ)
9+
select pred, succ

‎javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| electron.js:39:1:39:19 | foo(bw).webContents |
2+
| electron.js:40:1:40:19 | foo(bv).webContents |
3+
| electron.ts:4:3:4:16 | bw.webContents |
4+
| electron.ts:5:3:5:16 | bv.webContents |

‎javascript/ql/test/library-tests/frameworks/Electron/WebContents.ql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import javascript
2+
3+
from Electron::WebContents wc
4+
select wc

‎javascript/ql/test/library-tests/frameworks/Electron/WebPreferences.expected‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
| electron.js:3:36:3:37 | {} |
2-
| electron.js:4:34:4:35 | {} |
1+
| electron.js:3:45:3:46 | {} |
2+
| electron.js:4:43:4:44 | {} |

‎javascript/ql/test/library-tests/frameworks/Electron/electron.d.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
declare namespace Electron {
2+
export class BrowserWindow { }
3+
export class BrowserView { }
4+
}
5+
6+
declare module 'electron' {
7+
export = Electron;
8+
}

0 commit comments

Comments
 (0)