JS: polish js/enabling-electron-renderer-node-integration meta info

Author: Esben Sparre Andreasen

change-notes/1.19/analysis-javascript.md

@@ -12,6 +12,7 @@
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |
 |-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| Enabling Node.js integration for Electron web content renderers (`js/enabling-electron-renderer-node-integration`) | security, frameworks/electron, external/cwe/cwe-094  | Highlights Electron web content renderer preferences with Node.js integration enabled, indicating a violation of [CWE-94](https://cwe.mitre.org/data/definitions/94.html). Results are not shown on LGTM by default. |
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 
 ## Changes to existing queries

javascript/config/suites/javascript/security

@@ -1,4 +1,5 @@
 + semmlecode-javascript-queries/DOM/TargetBlank.ql: /Security/CWE/CWE-200
++ semmlecode-javascript-queries/Electron/EnablingNodeIntegration.ql: /Security/CWE/CWE-094
 + semmlecode-javascript-queries/Security/CWE-022/TaintedPath.ql: /Security/CWE/CWE-022
 + semmlecode-javascript-queries/Security/CWE-078/CommandInjection.ql: /Security/CWE/CWE-078
 + semmlecode-javascript-queries/Security/CWE-079/ReflectedXss.ql: /Security/CWE/CWE-079

javascript/ql/src/Electron/EnablingNodeIntegration.qhelp

@@ -5,39 +5,48 @@
 
   <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>
+
+	    Enabling Node.js integration in Electron web content renderers
+	    (<code>BrowserWindow</code>, <code>BrowserView</code> and
+	    <code>webview</code>) can result in remote native code execution
+	    attacks.
+
+	    The attack is realized when the renderer uses content from an
+	    untrusted remote web site or a trusted site with a cross site
+	    scripting vulnerability.
+
+        </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:
+
+	    Node.js integration should be disabled when loading remote web
+	    sites. Always set <code>nodeIntegration</code> preference
+	    to <code>false</code> before loading remote web sites, and only enable
+	    it for whitelisted sites.
+
     </p>
-    <sample src="examples/DefaultNodeIntegration.js"/>
 
     <p>
-      This is problematic, because default value of <code>nodeIntegration</code> is 'true'.
+
+	    Note that the <code>nodeIntegration</code> property is enabled
+	    by default in Electron and needs to be set to <code>false</code>
+	    explicitly.
+
     </p>
-    
-  </example>
+  </recommendation>
 
-
   <example>
+
     <p>
-      The following example shows insecure and secure uses of <code>BrowserWindow</code> and <code>BrowserView</code> when
-      loading untrusted web sites:
+
+	    The following examples shows insecure and secure uses of
+	    <code>BrowserWindow</code> and <code>BrowserView</code> when loading
+	    remote web sites:
+
     </p>
+
     <sample src="examples/EnablingNodeIntegration.js"/>
 
   </example>

javascript/ql/src/Electron/EnablingNodeIntegration.ql

@@ -1,11 +1,13 @@
 /**
- * @name Enabling `nodeIntegration` or `nodeIntegrationInWorker` for Electron web content
+ * @name Enabling Node.js integration for Electron web content renderers
  * @description Enabling `nodeIntegration` or `nodeIntegrationInWorker` can expose the application to remote code execution.
  * @kind problem
  * @problem.severity warning
+ * @precision low
  * @id js/enabling-electron-renderer-node-integration
  * @tags security
  *       frameworks/electron
+ *       external/cwe/cwe-094
  */
 
 import javascript

javascript/ql/src/Electron/examples/DefaultNodeIntegration.js

@@ -1,26 +1,21 @@
-//BAD
-win_1 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: true}});
-win_1.loadURL("https://untrusted-site.com");
+//BAD: `nodeIntegration` enabled by default
+var win_1 = new BrowserWindow();
+win_1.loadURL(remote_site);
 
-//GOOD
-win_2 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}});
-win_2.loadURL("https://untrusted-site.com");
+//BAD: `nodeIntegration` enabled
+var win_2 = new BrowserWindow({webPreferences: {nodeIntegration: true}});
+win_2.loadURL(remote_site);
 
-//BAD
-win_3 = new BrowserWindow({
-    webPreferences: {
-      nodeIntegrationInWorker: true
-    }
-});
+//GOOD: `nodeIntegration` disabled
+let win_3 = new BrowserWindow({webPreferences: {nodeIntegration: false}});
+win_3.loadURL(remote_site);
 
-//BAD BrowserView
-win_4 = new BrowserWindow({width: 800, height: 600, webPreferences: {nodeIntegration: false}})
-view = new BrowserView({
+//BAD: `nodeIntegration` enabled  in the view
+var win_4 = new BrowserWindow({webPreferences: {nodeIntegration: false}})
+var view_4 = 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');
-
+win_4.setBrowserView(view_4);
+view_4.webContents.loadURL(remote_site);