Skip to content

Commit 9179701

Browse files
committed
JavaScript: Add query for Node.js integration in Electron framework
1 parent ec62657 commit 9179701

File tree

8 files changed

+184
-0
lines changed

8 files changed

+184
-0
lines changed
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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 (BrowserWindow, BrowserView and webview) 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 the web content under processing. Please note that
11+
the nodeIntegration property is enabled by default in Electron and needs to be set to 'false' 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 BrowserWindow 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 nodeIntegration is 'true'.
31+
</p>
32+
33+
</example>
34+
35+
<example>
36+
<p>
37+
The following example shows insecure and secure uses of <webview> tag:
38+
</p>
39+
<sample src="examples/WebViewNodeIntegration.html"/>
40+
41+
</example>
42+
43+
<example>
44+
<p>
45+
The following example shows insecure and secure uses of BrowserWindow and BrowserView when
46+
loading untrusted web sites:
47+
</p>
48+
<sample src="examples/EnablingNodeIntegration.js"/>
49+
50+
</example>
51+
52+
53+
<references>
54+
<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>
55+
</references>
56+
</qhelp>
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* @name Enabling nodeIntegration and nodeIntegrationInWorker in webPreferences
3+
* @description Enabling nodeIntegration and nodeIntegrationInWorker could expose your app to remote code execution.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @precision very-high
7+
* @tags security
8+
* frameworks/electron
9+
* @id js/enabling-electron-renderer-node-integration
10+
*/
11+
12+
import javascript
13+
14+
string checkWebOptions(DataFlow::PropWrite prop, Electron::WebPreferences pref) {
15+
(prop = pref.getAPropertyWrite("nodeIntegration") and
16+
prop.getRhs().mayHaveBooleanValue(true) and
17+
result = "nodeIntegration property may have been enabled on this object that could result in RCE")
18+
or
19+
(prop = pref.getAPropertyWrite("nodeIntegrationInWorker") and
20+
prop.getRhs().mayHaveBooleanValue(true) and
21+
result = "nodeIntegrationInWorker property may have been enabled on this object that could result in RCE")
22+
or
23+
(not exists(pref.asExpr().(ObjectExpr).getPropertyByName("nodeIntegration")) and
24+
result = "nodeIntegration is enabled by default in WebPreferences object that could result in RCE")
25+
}
26+
27+
from DataFlow::PropWrite property, Electron::WebPreferences preferences
28+
select preferences,checkWebOptions(property, preferences)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const win = new BrowserWindow();
2+
win.loadURL("https://untrusted-site.com");
Lines changed: 26 additions & 0 deletions
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+
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset = "UTF-8">
5+
<title>WebView Examples</title>
6+
</head>
7+
8+
<body>
9+
<!-- BAD -->
10+
<webview src="https://untrusted-site.com/" nodeintegration></webview>
11+
12+
<!-- GOOD -->
13+
<webview src="https://untrusted-site.com/"></webview>
14+
</body>
15+
</html>
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| EnablingNodeIntegration.js:5:28:11:9 | {\\r\\n ... } | nodeIntegration property may have been enabled on this object that could result in RCE |
2+
| EnablingNodeIntegration.js:5:28:11:9 | {\\r\\n ... } | nodeIntegrationInWorker property may have been enabled on this object that could result in RCE |
3+
| EnablingNodeIntegration.js:15:22:20:9 | {\\r\\n ... } | nodeIntegration is enabled by default in WebPreferences object that could result in RCE |
4+
| EnablingNodeIntegration.js:23:13:27:9 | {\\r\\n ... } | nodeIntegration is enabled by default in WebPreferences object that could result in RCE |
5+
| EnablingNodeIntegration.js:49:71:49:93 | {nodeIn ... : true} | nodeIntegration property may have been enabled on this object that could result in RCE |
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
const {BrowserWindow} = require('electron')
2+
3+
function test() {
4+
var unsafe_1 = {
5+
webPreferences: {
6+
nodeIntegration: true,
7+
nodeIntegrationInWorker: true,
8+
plugins: true,
9+
webSecurity: true,
10+
sandbox: true
11+
}
12+
};
13+
14+
var options_1 = {
15+
webPreferences: {
16+
plugins: true,
17+
nodeIntegrationInWorker: false,
18+
webSecurity: true,
19+
sandbox: true
20+
}
21+
};
22+
23+
var pref = {
24+
plugins: true,
25+
webSecurity: true,
26+
sandbox: true
27+
};
28+
29+
var options_2 = {
30+
webPreferences: pref,
31+
show: true,
32+
frame: true,
33+
minWidth: 300,
34+
minHeight: 300
35+
};
36+
37+
var safe_used = {
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}});
50+
var w5 = new BrowserWindow(options_2);
51+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../../../src/Electron/EnablingNodeIntegration.ql

0 commit comments

Comments
 (0)