Skip to content

JavaScript: Add query for Node.js integration in Electron framework #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 15, 2018
49 changes: 49 additions & 0 deletions javascript/ql/src/Electron/EnablingNodeIntegration.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<p>
Enabling Node.js integration in web content renderers (<code>BrowserWindow</code>, <code>BrowserView</code> and <code>webview</code>) could result in
remote native code execution attacks when rendering malicious JavaScript code from untrusted remote web site or
code that is injected via a cross site scripting vulnerability into a trusted remote web site. Note that
the <code>nodeIntegration</code> property is enabled by default in Electron and needs to be set to <code>false</code> explicitly.
</p>
</overview>

<recommendation>
<p>
Node.js integration should be disabled when loading remote web sites. If not possible, always set nodeIntegration property
to 'false' before loading remote web sites and only enable it for whitelisted sites.
</p>
</recommendation>

<example>
<p>
The following example shows insecure use of <code>BrowserWindow</code> with regards to <code>nodeIntegration</code>
property:
</p>
<sample src="examples/DefaultNodeIntegration.js"/>

<p>
This is problematic, because default value of <code>nodeIntegration</code> is 'true'.
</p>

</example>


<example>
<p>
The following example shows insecure and secure uses of <code>BrowserWindow</code> and <code>BrowserView</code> when
loading untrusted web sites:
</p>
<sample src="examples/EnablingNodeIntegration.js"/>

</example>


<references>
<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>
</references>
</qhelp>
32 changes: 32 additions & 0 deletions javascript/ql/src/Electron/EnablingNodeIntegration.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* @name Enabling `nodeIntegration` or `nodeIntegrationInWorker` for Electron web content
* @description Enabling `nodeIntegration` or `nodeIntegrationInWorker` can expose the application to remote code execution.
* @kind problem
* @problem.severity warning
* @id js/enabling-electron-renderer-node-integration
* @tags security
* frameworks/electron
*/

import javascript

/**
* Gets a warning message for `pref` if one of the `nodeIntegration` features is enabled.
*/
string getNodeIntegrationWarning(Electron::WebPreferences pref) {
exists (string feature |
feature = "nodeIntegration" or
feature = "nodeIntegrationInWorker" |
pref.getAPropertyWrite(feature).getRhs().mayHaveBooleanValue(true) and
result = "The `" + feature + "` feature has been enabled."
)
or
exists (string feature |
feature = "nodeIntegration" |
not exists(pref.getAPropertyWrite(feature)) and
result = "The `" + feature + "` feature is enabled by default."
)
}

from Electron::WebPreferences preferences
select preferences, getNodeIntegrationWarning(preferences)
2 changes: 2 additions & 0 deletions javascript/ql/src/Electron/examples/DefaultNodeIntegration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const win = new BrowserWindow();
win.loadURL("https://untrusted-site.com");
26 changes: 26 additions & 0 deletions javascript/ql/src/Electron/examples/EnablingNodeIntegration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//BAD
win_1 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: true}});
win_1.loadURL("https://untrusted-site.com");

//GOOD
win_2 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}});
win_2.loadURL("https://untrusted-site.com");

//BAD
win_3 = new BrowserWindow({
webPreferences: {
nodeIntegrationInWorker: true
}
});

//BAD BrowserView
win_4 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}})
view = new BrowserView({
webPreferences: {
nodeIntegration: true
}
});
win.setBrowserView(view);
view.setBounds({ x: 0, y: 0, width: 300, height: 300 });
view.webContents.loadURL('https://untrusted-site.com');

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
| EnablingNodeIntegration.js:5:28:11:9 | {\\n ... } | The `nodeIntegrationInWorker` feature has been enabled. |
| EnablingNodeIntegration.js:5:28:11:9 | {\\n ... } | The `nodeIntegration` feature has been enabled. |
| EnablingNodeIntegration.js:15:22:20:9 | {\\n ... } | The `nodeIntegration` feature is enabled by default. |
| EnablingNodeIntegration.js:23:16:27:9 | { // NO ... } | The `nodeIntegration` feature is enabled by default. |
| EnablingNodeIntegration.js:49:74:49:96 | {nodeIn ... : true} | The `nodeIntegration` feature has been enabled. |
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
const {BrowserWindow} = require('electron')

function test() {
var unsafe_1 = { // NOT OK, both enabled
webPreferences: {
nodeIntegration: true,
nodeIntegrationInWorker: true,
plugins: true,
webSecurity: true,
sandbox: true
}
};

var options_1 = { // NOT OK, `nodeIntegrationInWorker` enabled
webPreferences: {
plugins: true,
nodeIntegrationInWorker: false,
webSecurity: true,
sandbox: true
}
};

var pref = { // NOT OK, implicitly enabled
plugins: true,
webSecurity: true,
sandbox: true
};

var options_2 = { // NOT OK, implicitly enabled
webPreferences: pref,
show: true,
frame: true,
minWidth: 300,
minHeight: 300
};

var safe_used = { // NOT OK, explicitly disabled
webPreferences: {
nodeIntegration: false,
plugins: true,
webSecurity: true,
sandbox: true
}
};

var w1 = new BrowserWindow(unsafe_1);
var w2 = new BrowserWindow(options_1);
var w3 = new BrowserWindow(safe_used);
var w4 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: true}}); // NOT OK, `nodeIntegration` enabled
var w5 = new BrowserWindow(options_2);
var w6 = new BrowserWindow(safe_used);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Electron/EnablingNodeIntegration.ql