Skip to content
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

JS/TS: insecure Helmet middleware (new query) #16540

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3a885ea
Insecure Helmet middle configuration - frameguard or CSP to 'false'
aegilops May 20, 2024
8300aeb
Tests for InsecureHelmet
aegilops May 20, 2024
83037b1
Adjust structure to avoid warnings about message
aegilops May 21, 2024
bda794f
Fixed wrong filenames in the InsecureHelmet tests
aegilops May 21, 2024
68e21a5
Fixed query help formatting issues
aegilops May 21, 2024
65dfd4c
Merge branch 'main' into aegilops/js/insecure-helmet-middleware
aegilops May 21, 2024
f5d465f
Added data extension to allow setting extra required Helmet features
aegilops Jun 7, 2024
29322f5
Merge branch 'aegilops/js/insecure-helmet-middleware' of https://gith…
aegilops Jun 7, 2024
465d64a
Removed br tags
aegilops Jun 7, 2024
7136763
Formatting
aegilops Jun 7, 2024
43a140e
Merge branch 'main' into aegilops/js/insecure-helmet-middleware
aegilops Jun 7, 2024
975811a
Change layout of qhelp example code
aegilops Jun 7, 2024
7ee5655
Merge branch 'aegilops/js/insecure-helmet-middleware' of https://gith…
aegilops Jun 7, 2024
da9e1e6
Moved examples into separate files
aegilops Jun 18, 2024
81ef255
Change to helmetProperty from helmetSetting variable name
aegilops Jun 19, 2024
f4691b1
Changed to more-modern Dataflow libraries
aegilops Jun 19, 2024
de96d39
Renamed to helmetProperty everywhere
aegilops Jun 19, 2024
8a3cec4
Fix formatting for check
aegilops Jun 19, 2024
d142f83
Change note and changed name of query in `.ql` file
aegilops Jun 19, 2024
3a98edb
Merge branch 'main' into aegilops/js/insecure-helmet-middleware
aegilops Jun 19, 2024
252c9e9
Added data extension to set defaults, updated help, added README to e…
aegilops Jun 19, 2024
26f1b36
Fixed formatting
aegilops Jun 19, 2024
a07639f
Set severity to 7.0, in line with other configuration queries
aegilops Jun 19, 2024
1ecd727
Renamed README to CUSTOMIZING, removed details from qhelp and referen…
aegilops Jun 19, 2024
f227789
Fixed expected test results for Helmet query
aegilops Jun 26, 2024
d1d0829
More external references
aegilops Jul 1, 2024
fc6fba8
Fixed CWE tags
aegilops Jul 1, 2024
c003f26
Fixed missing li closing tag
aegilops Jul 8, 2024
d896fdf
Merge branch 'main' into aegilops/js/insecure-helmet-middleware
aegilops Jul 8, 2024
2aff2a7
Fixed code markup
aegilops Jul 8, 2024
5a3328b
Merge branch 'aegilops/js/insecure-helmet-middleware' of https://gith…
aegilops Jul 8, 2024
412ad17
Merge branch 'main' into aegilops/js/insecure-helmet-middleware
aegilops Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
extensions:
aegilops marked this conversation as resolved.
Show resolved Hide resolved
- addsTo:
pack: codeql/javascript-queries
extensible: requiredHelmetSecuritySetting
data:
- ["frameguard"]
- ["contentSecurityPolicy"]
36 changes: 36 additions & 0 deletions javascript/ql/src/Security/CWE-693/CUSTOMIZING.md
Original file line number Diff line number Diff line change
@@ -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)
71 changes: 71 additions & 0 deletions javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
Original file line number Diff line number Diff line change
@@ -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>
aegilops marked this conversation as resolved.
Show resolved Hide resolved
<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>
47 changes: 47 additions & 0 deletions javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
Original file line number Diff line number Diff line change
@@ -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);
aegilops marked this conversation as resolved.
Show resolved Hide resolved

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()
10 changes: 10 additions & 0 deletions javascript/ql/src/Security/CWE-693/examples/helmet_custom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
app.use(
helmet({
contentSecurityPolicy: {
directives: {
"script-src": ["'self'", "example.com"],
"style-src": null,
},
},
})
);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
app.use(helmet());
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const helmet = require('helmet');

app.use(helmet({
frameguard: false,
contentSecurityPolicy: false
}));
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Security/CWE-693/InsecureHelmet.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const express = require("express");
const helmet = require("helmet");

const app = express();

app.use(helmet({
contentSecurityPolicy: false, // BAD: switch off default CSP
frameguard: false // BAD: switch off default frameguard
}));

app.get("/", (req, res) => {
res.send("Hello, world!");
});

app.listen(3000, () => {
console.log("App is listening on port 3000");
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const express = require("express");
const helmet = require("helmet");

const app = express();

app.use(helmet()); // GOOD: use the defaults

app.get("/", (req, res) => {
res.send("Hello, world!");
});

app.listen(3000, () => {
console.log("App is listening on port 3000");
});
Loading