Skip to content

Commit 782e91b

Browse files
authored
Merge pull request #167 from bnxi/NodeIntegration
Approved by esben-semmle
2 parents e4b9d31 + 7071c75 commit 782e91b

File tree

7 files changed

+167
-0
lines changed

7 files changed

+167
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Enabling Node.js integration in web content renderers (<code>BrowserWindow</code>, <code>BrowserView</code> and <code>webview</code>) could result in
9+
remote native code execution attacks when rendering malicious JavaScript code from untrusted remote web site or
10+
code that is injected via a cross site scripting vulnerability into a trusted remote web site. Note that
11+
the <code>nodeIntegration</code> property is enabled by default in Electron and needs to be set to <code>false</code> explicitly.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Node.js integration should be disabled when loading remote web sites. If not possible, always set nodeIntegration property
18+
to 'false' before loading remote web sites and only enable it for whitelisted sites.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
The following example shows insecure use of <code>BrowserWindow</code> with regards to <code>nodeIntegration</code>
25+
property:
26+
</p>
27+
<sample src="examples/DefaultNodeIntegration.js"/>
28+
29+
<p>
30+
This is problematic, because default value of <code>nodeIntegration</code> is 'true'.
31+
</p>
32+
33+
</example>
34+
35+
36+
<example>
37+
<p>
38+
The following example shows insecure and secure uses of <code>BrowserWindow</code> and <code>BrowserView</code> when
39+
loading untrusted web sites:
40+
</p>
41+
<sample src="examples/EnablingNodeIntegration.js"/>
42+
43+
</example>
44+
45+
46+
<references>
47+
<li>Electron Documentation: <a href="https://electronjs.org/docs/tutorial/security#2-disable-nodejs-integration-for-remote-content">Security, Native Capabilities, and Your Responsibility</a></li>
48+
</references>
49+
</qhelp>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* @name Enabling `nodeIntegration` or `nodeIntegrationInWorker` for Electron web content
3+
* @description Enabling `nodeIntegration` or `nodeIntegrationInWorker` can expose the application to remote code execution.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @id js/enabling-electron-renderer-node-integration
7+
* @tags security
8+
* frameworks/electron
9+
*/
10+
11+
import javascript
12+
13+
/**
14+
* Gets a warning message for `pref` if one of the `nodeIntegration` features is enabled.
15+
*/
16+
string getNodeIntegrationWarning(Electron::WebPreferences pref) {
17+
exists (string feature |
18+
feature = "nodeIntegration" or
19+
feature = "nodeIntegrationInWorker" |
20+
pref.getAPropertyWrite(feature).getRhs().mayHaveBooleanValue(true) and
21+
result = "The `" + feature + "` feature has been enabled."
22+
)
23+
or
24+
exists (string feature |
25+
feature = "nodeIntegration" |
26+
not exists(pref.getAPropertyWrite(feature)) and
27+
result = "The `" + feature + "` feature is enabled by default."
28+
)
29+
}
30+
31+
from Electron::WebPreferences preferences
32+
select preferences, getNodeIntegrationWarning(preferences)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const win = new BrowserWindow();
2+
win.loadURL("https://untrusted-site.com");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//BAD
2+
win_1 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: true}});
3+
win_1.loadURL("https://untrusted-site.com");
4+
5+
//GOOD
6+
win_2 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}});
7+
win_2.loadURL("https://untrusted-site.com");
8+
9+
//BAD
10+
win_3 = new BrowserWindow({
11+
webPreferences: {
12+
nodeIntegrationInWorker: true
13+
}
14+
});
15+
16+
//BAD BrowserView
17+
win_4 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}})
18+
view = new BrowserView({
19+
webPreferences: {
20+
nodeIntegration: true
21+
}
22+
});
23+
win.setBrowserView(view);
24+
view.setBounds({ x: 0, y: 0, width: 300, height: 300 });
25+
view.webContents.loadURL('https://untrusted-site.com');
26+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| EnablingNodeIntegration.js:5:28:11:9 | {\\n ... } | The `nodeIntegrationInWorker` feature has been enabled. |
2+
| EnablingNodeIntegration.js:5:28:11:9 | {\\n ... } | The `nodeIntegration` feature has been enabled. |
3+
| EnablingNodeIntegration.js:15:22:20:9 | {\\n ... } | The `nodeIntegration` feature is enabled by default. |
4+
| EnablingNodeIntegration.js:23:16:27:9 | { // NO ... } | The `nodeIntegration` feature is enabled by default. |
5+
| EnablingNodeIntegration.js:49:74:49:96 | {nodeIn ... : true} | The `nodeIntegration` feature has been enabled. |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
const {BrowserWindow} = require('electron')
2+
3+
function test() {
4+
var unsafe_1 = { // NOT OK, both enabled
5+
webPreferences: {
6+
nodeIntegration: true,
7+
nodeIntegrationInWorker: true,
8+
plugins: true,
9+
webSecurity: true,
10+
sandbox: true
11+
}
12+
};
13+
14+
var options_1 = { // NOT OK, `nodeIntegrationInWorker` enabled
15+
webPreferences: {
16+
plugins: true,
17+
nodeIntegrationInWorker: false,
18+
webSecurity: true,
19+
sandbox: true
20+
}
21+
};
22+
23+
var pref = { // NOT OK, implicitly enabled
24+
plugins: true,
25+
webSecurity: true,
26+
sandbox: true
27+
};
28+
29+
var options_2 = { // NOT OK, implicitly enabled
30+
webPreferences: pref,
31+
show: true,
32+
frame: true,
33+
minWidth: 300,
34+
minHeight: 300
35+
};
36+
37+
var safe_used = { // NOT OK, explicitly disabled
38+
webPreferences: {
39+
nodeIntegration: false,
40+
plugins: true,
41+
webSecurity: true,
42+
sandbox: true
43+
}
44+
};
45+
46+
var w1 = new BrowserWindow(unsafe_1);
47+
var w2 = new BrowserWindow(options_1);
48+
var w3 = new BrowserWindow(safe_used);
49+
var w4 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: true}}); // NOT OK, `nodeIntegration` enabled
50+
var w5 = new BrowserWindow(options_2);
51+
var w6 = new BrowserWindow(safe_used);
52+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Electron/EnablingNodeIntegration.ql

0 commit comments

Comments
 (0)