Merge pull request github#16 from rdmarsh/rdmarsh/js/electron-unsafe-web-preferences

JavaScript: Add query for disabling webSecurity in Electron

Author: rdmarsh2

change-notes/1.18/analysis-javascript.md

@@ -20,7 +20,8 @@
 
 | **Query**                   | **Tags**  | **Purpose**                                                        |
 |-----------------------------|-----------|--------------------------------------------------------------------|
-| Use of externally-controlled format string (`js/tainted-format-string`) | security, external/cwe/cwe-134 | Highlights format strings containing user-provided data, indicating a violation of [CWE-134](https://cwe.mitre.org/data/definitions/134.html). Results shown on lgtm by default. |
+| Disabling Electron webSecurity (`js/disabling-electron-websecurity`) | security, frameworks/electron | Highlights Electron browser objects that are created with the `webSecurity` property set to false. Results shown on LGTM by default. |
+| Use of externally-controlled format string (`js/tainted-format-string`) | security, external/cwe/cwe-134 | Highlights format strings containing user-provided data, indicating a violation of [CWE-134](https://cwe.mitre.org/data/definitions/134.html). Results shown on LGTM by default. |
 
 ## Changes to existing queries
 

javascript/config/suites/javascript/frameworks-more

@@ -21,3 +21,4 @@
 + semmlecode-javascript-queries/React/InconsistentStateUpdate.ql: /Frameworks/React
 + semmlecode-javascript-queries/React/UnsupportedStateUpdateInLifecycleMethod.ql: /Frameworks/React
 + semmlecode-javascript-queries/React/UnusedOrUndefinedStateProperty.ql: /Frameworks/React
++ semmlecode-javascript-queries/Electron/DisablingWebSecurity.ql: /Frameworks/Electron

javascript/ql/src/Electron/DisablingWebSecurity.qhelp

@@ -0,0 +1,41 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+  <overview>
+    <p>
+      Electron is secure by default through a same-origin policy requiring all
+      JavaScript and CSS code to originate from the machine running the
+      Electron application. Setting the <code>webSecurity</code> property of a
+      <code>webPreferences</code> object to <code>false</code> will disable the
+      same-origin policy.
+    </p>
+    <p>
+      Disabling the same-origin policy is strongly discouraged.
+    </p>
+  </overview>
+  
+  <recommendation>
+    <p>
+      Do not disable <code>webSecurity</code>.
+    </p>
+  </recommendation>
+  
+  <example>
+    <p>
+      The following example shows <code>webSecurity</code> being disabled.
+    </p>
+    <sample src="examples/DisablingWebSecurity.js"/>
+    
+    <p>
+      This is problematic, since it allows the execution of insecure code from
+      other domains.
+    </p>
+    
+  </example>
+  
+  <references>
+    <li>Electron Documentation: <a href="https://electronjs.org/docs/tutorial/security#5-do-not-disable-websecurity">Security, Native Capabilities, and Your Responsibility</a></li>
+  </references>
+</qhelp>

javascript/ql/src/Electron/DisablingWebSecurity.ql

@@ -0,0 +1,17 @@
+/**
+ * @name Disabling Electron webSecurity
+ * @description Disabling webSecurity can cause critical security vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @precision very-high
+ * @tags security
+ *       frameworks/electron
+ * @id js/disabling-electron-websecurity
+ */
+
+import javascript
+
+from DataFlow::PropWrite webSecurity, Electron::WebPreferences preferences
+where webSecurity = preferences.getAPropertyWrite("webSecurity")
+  and webSecurity.getRhs().mayHaveBooleanValue(false)
+select webSecurity, "Disabling webSecurity is strongly discouraged."

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

@@ -0,0 +1,5 @@
+const mainWindow = new BrowserWindow({
+  webPreferences: {
+    webSecurity: false
+  }
+})

javascript/ql/src/javascript.qll

@@ -52,6 +52,7 @@ import semmle.javascript.frameworks.Babel
 import semmle.javascript.frameworks.Credentials
 import semmle.javascript.frameworks.CryptoLibraries
 import semmle.javascript.frameworks.DigitalOcean
+import semmle.javascript.frameworks.Electron
 import semmle.javascript.frameworks.jQuery
 import semmle.javascript.frameworks.LodashUnderscore
 import semmle.javascript.frameworks.HttpFrameworks

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

@@ -0,0 +1,36 @@
+import javascript
+
+module Electron {
+  /**
+   * A data flow node that is an Electron `webPreferences` property.
+   */
+  class WebPreferences extends DataFlow::ObjectLiteralNode {
+    WebPreferences() {
+      exists(BrowserObject bo | this = bo.getOptionArgument(0, "webPreferences").getALocalSource())
+    }
+  }
+  
+  /**
+   * A data flow node that creates a new `BrowserWindow` or `BrowserView`.
+   */
+  private abstract class BrowserObject extends DataFlow::NewNode {
+  }
+  
+  /**
+   * A data flow node that creates a new `BrowserWindow`.
+   */
+  class BrowserWindow extends BrowserObject {
+    BrowserWindow() {
+      this = DataFlow::moduleMember("electron", "BrowserWindow").getAnInstantiation()
+    }
+  }
+  
+  /**
+   * A data flow node that creates a new `BrowserView`.
+   */
+  class BrowserView extends BrowserObject {
+    BrowserView() {
+      this = DataFlow::moduleMember("electron", "BrowserView").getAnInstantiation()
+    }
+  }
+}

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

@@ -0,0 +1,2 @@
+| electron.js:3:36:3:37 | {} |
+| electron.js:4:34:4:35 | {} |

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

@@ -0,0 +1,4 @@
+import javascript
+
+from Electron::WebPreferences wp
+select wp

javascript/ql/test/library-tests/frameworks/Electron/electron.js

@@ -0,0 +1,4 @@
+const {BrowserView, BrowserWindow} = require('electron')
+
+new BrowserWindow({webPreferences: {}})
+new BrowserView({webPreferences: {}})

javascript/ql/test/violation-tests/Electron/DangerousWebPreferencesSettings/DangerousWebPreferences.js

@@ -0,0 +1,36 @@
+const {BrowserWindow} = require('electron')
+
+function test() {
+    var unsafe_used = {
+        webPreferences: {
+            webSecurity: false,
+            allowRunningInsecureContent: true,
+            experimentalFeatures: true,
+            enableBlinkFeatures: ['ExecCommandInJavaScript'],
+            blinkFeatures: 'CSSVariables'
+        }
+    };
+    
+    var unsafe_unused = {
+        webPreferences: {
+            webSecurity: false,
+            allowRunningInsecureContent: true,
+            experimentalFeatures: true,
+            enableBlinkFeatures: ['ExecCommandInJavaScript'],
+            blinkFeatures: 'CSSVariables'
+        }
+    };
+    
+    var safe_used = {
+        webPreferences: {
+            webSecurity: true,
+            allowRunningInsecureContent: false,
+            experimentalFeatures: false,
+            enableBlinkFeatures: [],
+            blinkFeatures: ''
+        }
+    };
+    
+    new BrowserWindow(unsafe_used);
+    new BrowserWindow(safe_used);
+}

javascript/ql/test/violation-tests/Electron/DangerousWebPreferencesSettings/DisablingWebSecurity.expected

@@ -0,0 +1 @@
+| DangerousWebPreferences.js:6:13:6:30 | webSecurity: false | Disabling webSecurity is strongly discouraged. |

javascript/ql/test/violation-tests/Electron/DangerousWebPreferencesSettings/DisablingWebSecurity.qlref

@@ -0,0 +1 @@
+Electron/DisablingWebSecurity.ql