JS/TS: insecure Helmet middleware (new query)

[aegilops]

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 🚫.

I've included a new data extension, specific to this query, called requiredHelmetSecuritySetting, which users can set to a string of extra features that should not be disabled, beyond the defaults in the query. The extension is specific to this query, rather than general purpose; I don't know of other similar queries for other frameworks, so designing something that would work well in other contexts would be speculative.

I'd like to discuss 🗣️ what numeric severity 🔢 to give this, since it's a configuration and not a direct vulnerability on its own.

[github-actions]

QHelp previews:

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

errors/warnings:

/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:9:17: element "ul" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:22:17: element "ul" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:31:47: found attribute "class", but no attributes allowed here
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:42:47: found attribute "class", but no attributes allowed here
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:48:47: element "code" not allowed here; expected the element end-tag or text
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:48:47: found attribute "class", but no attributes allowed here
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:60:12: element "p" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp:63:7: The element type "code" must be terminated by the matching end-tag "</code>".
A fatal error occurred: 1 qhelp files could not be processed.

[github-actions]

QHelp previews:

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

Insecure configuration of Helmet security middleware

Helmet 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:

• Disabling frame protection
• Disabling Content Security Policy
  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.

Recommendation

To help mitigate these vulnerabilities, ensure that the following Helmet functions are not disabled, and are configured appropriately to your application:

• frameguard
• contentSecurityPolicy

Example

The following code snippet demonstrates Helmet configured in an insecure manner:

            const helmet = require('helmet');
            app.use(helmet({
                frameguard: false,
                contentSecurityPolicy: false
            }));
        

In this example, the defaults are used, which enables frame protection and a default Content Security Policy.

            app.use(helmet());
        

You can also enable a custom Content Security Policy by passing an object to the contentSecurityPolicy key. For example, taken from the Helmet docs:

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

References

• helmet.js website

[github-actions]

QHelp previews:

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

Insecure configuration of Helmet security middleware

Helmet 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:

• Disabling frame protection
• Disabling Content Security Policy
  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.

Users of the query can extend the set of required Helmet features by adding additional checks for them, using CodeQL data extensions in a CodeQL model pack. See `CUSTOMIZING.md` in the query source for more information.

Recommendation

To help mitigate these vulnerabilities, ensure that the following Helmet functions are not disabled, and are configured appropriately to your application:

• frameguard
• contentSecurityPolicy

Example

The following code snippet demonstrates Helmet configured in an insecure manner:

const helmet = require('helmet');

app.use(helmet({
    frameguard: false,
    contentSecurityPolicy: false
}));

In this example, the defaults are used, which enables frame protection and a default Content Security Policy.

app.use(helmet());

You can also enable a custom Content Security Policy by passing an object to the contentSecurityPolicy key. For example, taken from the Helmet docs:

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

References

• helmet.js website

[aegilops]

I'm not sure what to do about the change note failure. Do I need to document this change in a change note, and if so, where please?

[erik-krogh]

Do I need to document this change in a change note, and if so, where please?

Yes.
javascript/ql/src/change-notes/yyyy-mm-dd-name-of-your-node.md.

See this doc on change-notes for what to put there: https://github.com/github/codeql/blob/main/docs/change-notes.md

[erik-krogh]

Some comments for now.

[aegilops]

I've got those updates done for you @erik-krogh, thanks so much for the review so far 🙏

• added a change note
• switched to modern dataflow libraries you suggested
• used a default data extension, since using an external predicate always requires external data
• added a CUSTOMIZING.md to explain how to customize the query with data extensions
• fixed the inconsistent naming
• updated the severity to 7.0, as we discussed
• moved the examples to separate files outside the .qhelp file
• updated the name to js/... (instead of javascript/...), which I think is the convention

[erik-krogh]

You need to update the expected output of the test, and I think we could use some more references in the QHelp.

But the rest looks good 👍

[github-actions]

QHelp previews:

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

Insecure configuration of Helmet security middleware

Helmet 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:

• Disabling frame protection
• Disabling Content Security Policy
  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.

Users of the query can extend the set of required Helmet features by adding additional checks for them, using CodeQL data extensions in a CodeQL model pack. See CUSTOMIZING.md in the query source for more information.

Recommendation

To help mitigate these vulnerabilities, ensure that the following Helmet functions are not disabled, and are configured appropriately to your application:

• frameguard
• contentSecurityPolicy

Example

The following code snippet demonstrates Helmet configured in an insecure manner:

const helmet = require('helmet');

app.use(helmet({
    frameguard: false,
    contentSecurityPolicy: false
}));

In this example, the defaults are used, which enables frame protection and a default Content Security Policy.

app.use(helmet());

You can also enable a custom Content Security Policy by passing an object to the contentSecurityPolicy key. For example, taken from the Helmet docs:

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

References

• helmet.js website
• Content Security Policy (CSP) | MDN
• Mozilla Web Security Guidelines
• Protect against clickjacking | MDN
• Common Weakness Enumeration: CWE-693.
• Common Weakness Enumeration: CWE-1021.

[aegilops]

👋 I fixed the expected result, added more references, and fixed a bit of markup in the query help.

I just updated my branch with main and pushed my changes, so I hope this will be a clean run and OK to merge.

[erik-krogh]

Nice, this looks good to me 👍

One below comment about dataExtensions:. I don't know what the right answer is 🤷

I did some evaluations to check that the results/performance look OK.
(nightly, default (internal links)).

Performance was great 👍
And the evaluations found a single new TP, which looked like the below, so that's nice.

app.use(helmet({
  contentSecurityPolicy: false, // @TODO implement

[aegilops]

One future enhancement would be to look for places where a config is defined that flows to the app use.

I'm not going to do that right now, since I don't know if it is done in real use, but it's a possibility. I wanted to keep the initial implementation simple and performant, and to work for cases I saw in real code.

There may also be other variations in how the middleware is activated, which I can look out for in future.