Merge pull request github#16540 from aegilops/aegilops/js/insecure-helmet-middleware

JS/TS: insecure Helmet middleware (new query)

Author: erik-krogh

javascript/ql/lib/semmle/javascript/frameworks/helmet/Helmet.Required.Setting.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/javascript-queries
+      extensible: requiredHelmetSecuritySetting
+    data:
+      - ["frameguard"]
+      - ["contentSecurityPolicy"]

javascript/ql/src/Security/CWE-693/CUSTOMIZING.md

@@ -0,0 +1,36 @@
+# Insecure Helmet Configuration - customizations
+
+You can extend the required [Helmet security settings](https://helmetjs.github.io/) using [data extensions](https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/) in a [CodeQL model pack](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack).
+
+They are defaulted to just `frameguard` and `contentSecurityPolicy`, but you can add more using this method, to require them not to be set to `false` (which explicitly disables them) in the Helmet configuration.
+
+For example, this YAML model can be used inside a CodeQL model pack to require `frameguard` and `contentSecurityPolicy`:
+
+```yaml
+extensions:
+  - addsTo:
+      pack: codeql/javascript-all
+      extensible: requiredHelmetSecuritySetting
+    data:
+      - ["frameguard"]
+      - ["contentSecurityPolicy"]
+```
+
+Note: Using `frameguard` and `contentSecurityPolicy` is an example: the query already enforces these, so it is not necessary to add it with your own data extension.
+
+A suitable [model pack](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack) might be:
+
+```yaml
+name: my-org/javascript-helmet-insecure-config-model-pack
+version: 1.0.0
+extensionTargets:
+  codeql/java-all: '*'
+dataExtensions:
+  - models/**/*.yml
+```
+
+## References
+
+- [Helmet security settings](https://helmetjs.github.io/)
+- [Customizing library models for javascript](https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/)
+- [Creating and working with CodeQL packs](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack)

javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp

@@ -0,0 +1,71 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+    <overview>
+        <p>
+            <a href="https://helmetjs.github.io/">Helmet</a> is a collection of middleware functions for securing Express apps. It sets various HTTP headers to guard against common web vulnerabilities.
+
+            This query detects Helmet misconfigurations that can lead to security vulnerabilities, specifically:
+        </p>
+
+        <ul>
+            <li>Disabling frame protection</li>
+            <li>Disabling Content Security Policy</li>
+        </ul>
+
+        <p>
+            Content Security Policy (CSP) helps spot and prevent injection attacks such as Cross-Site Scripting (XSS).
+
+            Removing frame protections exposes an application to attacks such as clickjacking, where an attacker can trick a user into clicking on a button or link on a targeted page when they intended to click on the page carrying out the attack.
+        </p>
+
+        <p>
+            Users of the query can extend the set of required Helmet features by adding additional checks for them, using CodeQL <a href="https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/">data extensions</a> in a <a href="https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack">CodeQL model pack</a>. See <code>CUSTOMIZING.md</code> in the query source for more information.
+        </p>
+
+    </overview>
+    <recommendation>
+        <p>
+            To help mitigate these vulnerabilities, ensure that the following Helmet functions are not disabled, and are configured appropriately to your application:
+        </p>
+
+        <ul>
+            <li><code>frameguard</code></li>
+            <li><code>contentSecurityPolicy</code></li>
+        </ul>
+    </recommendation>
+    <example>
+        <p>
+            The following code snippet demonstrates Helmet configured in an insecure manner:
+        </p>
+
+        <sample src="examples/helmet_insecure.js" />
+
+        <p>
+            In this example, the defaults are used, which enables frame protection and a default Content Security Policy.
+        </p>
+
+        <sample src="examples/helmet_default.js" />
+
+        <p>
+            You can also enable a custom Content Security Policy by passing an object to the <code>contentSecurityPolicy</code> key. For example, taken from the <a href="https://helmetjs.github.io/#content-security-policy">Helmet docs</a>:
+        </p>
+
+        <sample src="examples/helmet_custom.js" />
+
+    </example>
+    <references>
+        <li>
+            <a href="https://helmetjs.github.io/">helmet.js website</a>
+        </li>
+        <li>
+            <a href="https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy">Content Security Policy (CSP) | MDN</a>
+        </li>
+        <li>
+            <a href="https://infosec.mozilla.org/guidelines/web_security">Mozilla Web Security Guidelines</a>
+        </li>
+        <li>
+            <a href="https://developer.mozilla.org/en-US/docs/Web/Security#protect_against_clickjacking">Protect against clickjacking | MDN</a>
+        </li>
+
+    </references>
+</qhelp>

javascript/ql/src/Security/CWE-693/InsecureHelmet.ql

@@ -0,0 +1,47 @@
+/**
+ * @name Insecure configuration of Helmet security middleware
+ * @description The Helmet middleware is used to set security-related HTTP headers in Express applications. This query finds instances where the middleware is configured with important security features disabled.
+ * @kind problem
+ * @problem.severity error
+ * @security-severity 7.0
+ * @precision high
+ * @id js/insecure-helmet-configuration
+ * @tags security
+ *        external/cwe/cwe-693
+ *        external/cwe/cwe-1021
+ */
+
+import javascript
+import DataFlow
+import semmle.javascript.frameworks.ExpressModules
+
+class HelmetProperty extends DataFlow::Node instanceof DataFlow::PropWrite {
+  ExpressLibraries::HelmetRouteHandler helmet;
+
+  HelmetProperty() {
+    this = helmet.(DataFlow::CallNode).getAnArgument().getALocalSource().getAPropertyWrite()
+  }
+
+  ExpressLibraries::HelmetRouteHandler getHelmet() { result = helmet }
+
+  predicate isFalse() { DataFlow::PropWrite.super.getRhs().mayHaveBooleanValue(false) }
+
+  string getName() { result = DataFlow::PropWrite.super.getPropertyName() }
+
+  predicate isImportantSecuritySetting() {
+    // read from data extensions to allow enforcing custom settings
+    // defaults are located in javascript/ql/lib/semmle/frameworks/helmet/Helmet.Required.Setting.model.yml
+    requiredHelmetSecuritySetting(this.getName())
+  }
+}
+
+extensible predicate requiredHelmetSecuritySetting(string name);
+
+from HelmetProperty helmetProperty, ExpressLibraries::HelmetRouteHandler helmet
+where
+  helmetProperty.isFalse() and
+  helmetProperty.isImportantSecuritySetting() and
+  helmetProperty.getHelmet() = helmet
+select helmet,
+  "Helmet security middleware, configured with security setting $@ set to 'false', which disables enforcing that feature.",
+  helmetProperty, helmetProperty.getName()

javascript/ql/src/Security/CWE-693/examples/helmet_custom.js

@@ -0,0 +1,10 @@
+app.use(
+    helmet({
+        contentSecurityPolicy: {
+            directives: {
+                "script-src": ["'self'", "example.com"],
+                "style-src": null,
+            },
+        },
+    })
+);

javascript/ql/src/Security/CWE-693/examples/helmet_default.js

@@ -0,0 +1 @@
+app.use(helmet());

javascript/ql/src/Security/CWE-693/examples/helmet_insecure.js

@@ -0,0 +1,6 @@
+const helmet = require('helmet');
+
+app.use(helmet({
+    frameguard: false,
+    contentSecurityPolicy: false
+}));

javascript/ql/src/change-notes/2024-06-19-insecure-helmet-config.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `js/insecure-helmet-configuration`, to detect instances where Helmet middleware is configured with important security features disabled.

javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.expected

@@ -0,0 +1,2 @@
+| InsecureHelmetBad.js:6:9:9:2 | helmet( ... uard\\n}) | Helmet security middleware, configured with security setting $@ set to 'false', which disables enforcing that feature. | InsecureHelmetBad.js:7:5:7:32 | content ... : false | contentSecurityPolicy |
+| InsecureHelmetBad.js:6:9:9:2 | helmet( ... uard\\n}) | Helmet security middleware, configured with security setting $@ set to 'false', which disables enforcing that feature. | InsecureHelmetBad.js:8:5:8:21 | frameguard: false | frameguard |

javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.qlref

@@ -0,0 +1 @@
+Security/CWE-693/InsecureHelmet.ql