diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index ca27b717f18..b8e3bb08131 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -27,10 +27,27 @@ class HelmetProperty extends Property {
predicate isImportantSecuritySetting() {
this.getName() in ["frameguard", "contentSecurityPolicy"]
// read from data extensions to allow enforcing other settings
- // TODO
+ or requiredHelmetSecuritySetting(this.getName())
}
}
+/*
+ * Extend the required Helmet security settings using data extensions.
+ * Docs: https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/
+ * For example:
+
+extensions:
+ - addsTo:
+ pack: codeql/javascript-all
+ extensible: requiredHelmetSecuritySetting
+ data:
+ - name: "frameguard"
+
+ * Note: `frameguard` is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
+
+ */
+extensible predicate requiredHelmetSecuritySetting(string name);
+
from HelmetProperty helmetSetting, ExpressLibraries::HelmetRouteHandler helmet
where
helmetSetting.isFalse() and
From 465d64a810a1d77cccfd6f1d63335530e9967959 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Fri, 7 Jun 2024 15:34:45 +0100
Subject: [PATCH 07/70] Removed br tags
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index d0550823ab6..8c0484d8a9e 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -2,7 +2,7 @@
- Helmet is a collection of middleware functions for securing Express apps. It sets various HTTP headers to guard against common web vulnerabilities.
+ 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:
@@ -13,7 +13,7 @@
- Content Security Policy (CSP) helps spot and prevent injection attacks such as Cross-Site Scripting (XSS).
+ 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.
From 7136763c37c4f993fde037df7c44bc9c1133fec5 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Fri, 7 Jun 2024 15:36:39 +0100
Subject: [PATCH 08/70] Formatting
---
.../ql/src/Security/CWE-693/InsecureHelmet.ql | 23 ++++++++++---------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index b8e3bb08131..70d4b41e096 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -26,8 +26,9 @@ class HelmetProperty extends Property {
predicate isImportantSecuritySetting() {
this.getName() in ["frameguard", "contentSecurityPolicy"]
+ or
// read from data extensions to allow enforcing other settings
- or requiredHelmetSecuritySetting(this.getName())
+ requiredHelmetSecuritySetting(this.getName())
}
}
@@ -35,17 +36,17 @@ class HelmetProperty extends Property {
* Extend the required Helmet security settings using data extensions.
* Docs: https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/
* For example:
-
-extensions:
- - addsTo:
- pack: codeql/javascript-all
- extensible: requiredHelmetSecuritySetting
- data:
- - name: "frameguard"
-
- * Note: `frameguard` is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
-
+ *
+ * extensions:
+ * - addsTo:
+ * pack: codeql/javascript-all
+ * extensible: requiredHelmetSecuritySetting
+ * data:
+ * - name: "frameguard"
+ *
+ * Note: `frameguard` is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
*/
+
extensible predicate requiredHelmetSecuritySetting(string name);
from HelmetProperty helmetSetting, ExpressLibraries::HelmetRouteHandler helmet
From 975811ae5973f537ca785ef0c453fb4b166b1205 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Fri, 7 Jun 2024 15:50:06 +0100
Subject: [PATCH 09/70] Change layout of qhelp example code
---
.../src/Security/CWE-693/InsecureHelmet.qhelp | 44 +++++++++----------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index 8c0484d8a9e..b2f87f9aeac 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -23,12 +23,12 @@
- extensions:
- - addsTo:
- pack: codeql/javascript-all
- extensible: requiredHelmetSecuritySetting
- data:
- - name: "frameguard"
+extensions:
+- addsTo:
+ pack: codeql/javascript-all
+ extensible: requiredHelmetSecuritySetting
+data:
+ - name: "frameguard"
@@ -52,11 +52,11 @@
- const helmet = require('helmet');
- app.use(helmet({
- frameguard: false,
- contentSecurityPolicy: false
- }));
+const helmet = require('helmet');
+app.use(helmet({
+ frameguard: false,
+ contentSecurityPolicy: false
+}));
@@ -64,7 +64,7 @@
- app.use(helmet());
+app.use(helmet());
@@ -72,16 +72,16 @@
- app.use(
- helmet({
- contentSecurityPolicy: {
- directives: {
- "script-src": ["'self'", "example.com"],
- "style-src": null,
- },
- },
- })
- );
+app.use(
+ helmet({
+ contentSecurityPolicy: {
+ directives: {
+ "script-src": ["'self'", "example.com"],
+ "style-src": null,
+ },
+ },
+ })
+);
From da9e1e61a4efa16001e6bc57faaf318bbf595f94 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Tue, 18 Jun 2024 19:50:06 +0100
Subject: [PATCH 10/70] Moved examples into separate files
---
.../src/Security/CWE-693/InsecureHelmet.qhelp | 25 +++----------------
.../CWE-693/examples/helmet_custom.js | 10 ++++++++
.../CWE-693/examples/helmet_default.js | 1 +
.../CWE-693/examples/helmet_insecure.js | 6 +++++
4 files changed, 20 insertions(+), 22 deletions(-)
create mode 100644 javascript/ql/src/Security/CWE-693/examples/helmet_custom.js
create mode 100644 javascript/ql/src/Security/CWE-693/examples/helmet_default.js
create mode 100644 javascript/ql/src/Security/CWE-693/examples/helmet_insecure.js
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index b2f87f9aeac..f0813ffecd7 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -51,38 +51,19 @@ data:
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,
- },
- },
- })
-);
-
+
diff --git a/javascript/ql/src/Security/CWE-693/examples/helmet_custom.js b/javascript/ql/src/Security/CWE-693/examples/helmet_custom.js
new file mode 100644
index 00000000000..5b9e25033f2
--- /dev/null
+++ b/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,
+ },
+ },
+ })
+);
\ No newline at end of file
diff --git a/javascript/ql/src/Security/CWE-693/examples/helmet_default.js b/javascript/ql/src/Security/CWE-693/examples/helmet_default.js
new file mode 100644
index 00000000000..98936520dcb
--- /dev/null
+++ b/javascript/ql/src/Security/CWE-693/examples/helmet_default.js
@@ -0,0 +1 @@
+app.use(helmet());
\ No newline at end of file
diff --git a/javascript/ql/src/Security/CWE-693/examples/helmet_insecure.js b/javascript/ql/src/Security/CWE-693/examples/helmet_insecure.js
new file mode 100644
index 00000000000..62852b9f482
--- /dev/null
+++ b/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
+}));
\ No newline at end of file
From 81ef255a87553ef361cd8b6bca8e64262a3cde76 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 10:09:50 +0100
Subject: [PATCH 11/70] Change to helmetProperty from helmetSetting variable
name
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index 70d4b41e096..c4bb5b5ab52 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -49,7 +49,7 @@ class HelmetProperty extends Property {
extensible predicate requiredHelmetSecuritySetting(string name);
-from HelmetProperty helmetSetting, ExpressLibraries::HelmetRouteHandler helmet
+from HelmetProperty helmetProperty, ExpressLibraries::HelmetRouteHandler helmet
where
helmetSetting.isFalse() and
helmetSetting.isImportantSecuritySetting() and
From f4691b191934b853eddafb000a76087ac8d30367 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 10:11:06 +0100
Subject: [PATCH 12/70] Changed to more-modern Dataflow libraries
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index c4bb5b5ab52..90ac834575d 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -13,16 +13,18 @@
import semmle.javascript.frameworks.ExpressModules
-class HelmetProperty extends Property {
+class HelmetProperty extends DataFlow::Node instanceof DataFlow::PropWrite {
ExpressLibraries::HelmetRouteHandler helmet;
HelmetProperty() {
- helmet.(DataFlow::CallNode).getAnArgument().asExpr().(ObjectExpr).getAProperty() = this
+ this = helmet.(DataFlow::CallNode).getAnArgument().getALocalSource().getAPropertyWrite()
}
ExpressLibraries::HelmetRouteHandler getHelmet() { result = helmet }
- predicate isFalse() { this.getInit().(BooleanLiteral).getBoolValue() = false }
+ predicate isFalse() { DataFlow::PropWrite.super.getRhs().mayHaveBooleanValue(true) }
+
+ string getName() { result = DataFlow::PropWrite.super.getPropertyName() }
predicate isImportantSecuritySetting() {
this.getName() in ["frameguard", "contentSecurityPolicy"]
From de96d3951d288788519afff210ad183cf860574b Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 10:15:06 +0100
Subject: [PATCH 13/70] Renamed to helmetProperty everywhere
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index 90ac834575d..debcd9c3ddd 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -53,8 +53,8 @@ extensible predicate requiredHelmetSecuritySetting(string name);
from HelmetProperty helmetProperty, ExpressLibraries::HelmetRouteHandler helmet
where
- helmetSetting.isFalse() and
- helmetSetting.isImportantSecuritySetting() and
- helmetSetting.getHelmet() = helmet
-select helmet, "Helmet route handler, called with $@ set to 'false'.", helmetSetting,
- helmetSetting.getName()
+ helmetProperty.isFalse() and
+ helmetProperty.isImportantSecuritySetting() and
+ helmetProperty.getHelmet() = helmet
+select helmet, "Helmet route handler, called with $@ set to 'false'.", helmetProperty,
+helmetProperty.getName()
From 8a3cec49778fa97d572c2e0c46fe3867783efd43 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 11:38:20 +0100
Subject: [PATCH 14/70] Fix formatting for check
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index debcd9c3ddd..c1ff6ca3e39 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -57,4 +57,4 @@ where
helmetProperty.isImportantSecuritySetting() and
helmetProperty.getHelmet() = helmet
select helmet, "Helmet route handler, called with $@ set to 'false'.", helmetProperty,
-helmetProperty.getName()
+ helmetProperty.getName()
From d142f830da8097c28722b79ef2920b8a2e2545dc Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 12:04:32 +0100
Subject: [PATCH 15/70] Change note and changed name of query in `.ql` file
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 2 +-
.../ql/src/change-notes/2024-06-19-insecure-helmet-config.md | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
create mode 100644 javascript/ql/src/change-notes/2024-06-19-insecure-helmet-config.md
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index c1ff6ca3e39..3a2643d603e 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -5,7 +5,7 @@
* @problem.severity error
* @security-severity 5.0
* @precision high
- * @id javascript/insecure-helmet-configuration
+ * @id js/insecure-helmet-configuration
* @tags security
* cwe-693
* cwe-1021
diff --git a/javascript/ql/src/change-notes/2024-06-19-insecure-helmet-config.md b/javascript/ql/src/change-notes/2024-06-19-insecure-helmet-config.md
new file mode 100644
index 00000000000..bee7ccb8fb9
--- /dev/null
+++ b/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.
From 252c9e9416c9b923ac98bdf0a53c09ce32e9c6a9 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 17:27:17 +0100
Subject: [PATCH 16/70] Added data extension to set defaults, updated help,
added README to explain customization
---
.../helmet/Helmet.Required.Setting.model.yml | 7 ++++
.../src/Security/CWE-693/InsecureHelmet.qhelp | 12 +++----
.../ql/src/Security/CWE-693/InsecureHelmet.ql | 26 ++++----------
javascript/ql/src/Security/CWE-693/README.md | 36 +++++++++++++++++++
4 files changed, 54 insertions(+), 27 deletions(-)
create mode 100644 javascript/ql/lib/semmle/javascript/frameworks/helmet/Helmet.Required.Setting.model.yml
create mode 100644 javascript/ql/src/Security/CWE-693/README.md
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/helmet/Helmet.Required.Setting.model.yml b/javascript/ql/lib/semmle/javascript/frameworks/helmet/Helmet.Required.Setting.model.yml
new file mode 100644
index 00000000000..ab01ec5206d
--- /dev/null
+++ b/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"]
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index f0813ffecd7..c09978e13d1 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -22,14 +22,12 @@
Users of the query can extend the set of required Helmet features by adding additional checks for them, using CodeQL data extensions.
-
-extensions:
-- addsTo:
- pack: codeql/javascript-all
- extensible: requiredHelmetSecuritySetting
+ extensions:
+ - addsTo:
+ pack: codeql/javascript-all
+ extensible: requiredHelmetSecuritySetting
data:
- - name: "frameguard"
-
+ - ["frameguard"]
Note: frameguard is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index 3a2643d603e..b5495714ac7 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -11,6 +11,8 @@
* cwe-1021
*/
+import javascript
+import DataFlow
import semmle.javascript.frameworks.ExpressModules
class HelmetProperty extends DataFlow::Node instanceof DataFlow::PropWrite {
@@ -22,33 +24,17 @@ class HelmetProperty extends DataFlow::Node instanceof DataFlow::PropWrite {
ExpressLibraries::HelmetRouteHandler getHelmet() { result = helmet }
- predicate isFalse() { DataFlow::PropWrite.super.getRhs().mayHaveBooleanValue(true) }
+ predicate isFalse() { DataFlow::PropWrite.super.getRhs().mayHaveBooleanValue(false) }
string getName() { result = DataFlow::PropWrite.super.getPropertyName() }
predicate isImportantSecuritySetting() {
- this.getName() in ["frameguard", "contentSecurityPolicy"]
- or
- // read from data extensions to allow enforcing other settings
+ // 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())
}
}
-/*
- * Extend the required Helmet security settings using data extensions.
- * Docs: https://codeql.github.com/docs/codeql-language-guides/customizing-library-models-for-javascript/
- * For example:
- *
- * extensions:
- * - addsTo:
- * pack: codeql/javascript-all
- * extensible: requiredHelmetSecuritySetting
- * data:
- * - name: "frameguard"
- *
- * Note: `frameguard` is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
- */
-
extensible predicate requiredHelmetSecuritySetting(string name);
from HelmetProperty helmetProperty, ExpressLibraries::HelmetRouteHandler helmet
@@ -56,5 +42,5 @@ where
helmetProperty.isFalse() and
helmetProperty.isImportantSecuritySetting() and
helmetProperty.getHelmet() = helmet
-select helmet, "Helmet route handler, called with $@ set to 'false'.", helmetProperty,
+select helmet, "Helmet security middleware, configured with security setting $@ set to 'false', which disables enforcing that feature.", helmetProperty,
helmetProperty.getName()
diff --git a/javascript/ql/src/Security/CWE-693/README.md b/javascript/ql/src/Security/CWE-693/README.md
new file mode 100644
index 00000000000..0ca0afd74bb
--- /dev/null
+++ b/javascript/ql/src/Security/CWE-693/README.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/).
+
+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 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)
From 26f1b367363b6fc1f6abc2b051bb0b22c2cf033d Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 17:41:58 +0100
Subject: [PATCH 17/70] Fixed formatting
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index b5495714ac7..39159b72406 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -42,5 +42,6 @@ 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()
+select helmet,
+ "Helmet security middleware, configured with security setting $@ set to 'false', which disables enforcing that feature.",
+ helmetProperty, helmetProperty.getName()
From a07639f4f6750ddf4be63f6ced724d36b9ae7c66 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 17:43:41 +0100
Subject: [PATCH 18/70] Set severity to 7.0, in line with other configuration
queries
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.ql | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
index 39159b72406..c4437d4913d 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.ql
@@ -3,7 +3,7 @@
* @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 5.0
+ * @security-severity 7.0
* @precision high
* @id js/insecure-helmet-configuration
* @tags security
From 1ecd72727ddacb1380cb7c7698bfbaac398728de Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 19 Jun 2024 17:59:43 +0100
Subject: [PATCH 19/70] Renamed README to CUSTOMIZING, removed details from
qhelp and referenced md doc instead
---
.../Security/CWE-693/{README.md => CUSTOMIZING.md} | 4 ++--
.../ql/src/Security/CWE-693/InsecureHelmet.qhelp | 13 +------------
2 files changed, 3 insertions(+), 14 deletions(-)
rename javascript/ql/src/Security/CWE-693/{README.md => CUSTOMIZING.md} (78%)
diff --git a/javascript/ql/src/Security/CWE-693/README.md b/javascript/ql/src/Security/CWE-693/CUSTOMIZING.md
similarity index 78%
rename from javascript/ql/src/Security/CWE-693/README.md
rename to javascript/ql/src/Security/CWE-693/CUSTOMIZING.md
index 0ca0afd74bb..34ae2851a85 100644
--- a/javascript/ql/src/Security/CWE-693/README.md
+++ b/javascript/ql/src/Security/CWE-693/CUSTOMIZING.md
@@ -1,6 +1,6 @@
# 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/).
+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.
@@ -18,7 +18,7 @@ extensions:
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 might be:
+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
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index c09978e13d1..e294779d6b8 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -19,18 +19,7 @@
- Users of the query can extend the set of required Helmet features by adding additional checks for them, using CodeQL data extensions.
-
-
- extensions:
- - addsTo:
- pack: codeql/javascript-all
- extensible: requiredHelmetSecuritySetting
-data:
- - ["frameguard"]
-
-
- Note: frameguard is an example: the query already enforces this setting, so it is not necessary to add it to the data extension.
+ 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.
From b71ba7c30f47d7fa7901c71ee8a481b94236333b Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Tue, 28 May 2024 10:40:15 +0100
Subject: [PATCH 20/70] Move Header Write derrived concepts to Concepts
---
python/ql/lib/semmle/python/Concepts.qll | 48 ++++++++++++++++++
.../HttpHeaderInjectionCustomizations.qll | 50 -------------------
2 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll
index 029f13ee0c2..8e64deddb4e 100644
--- a/python/ql/lib/semmle/python/Concepts.qll
+++ b/python/ql/lib/semmle/python/Concepts.qll
@@ -1134,6 +1134,54 @@ module Http {
}
}
+ /** A key-value pair in a literal for a bulk header update, considered as a single header update. */
+ private class HeaderBulkWriteDictLiteral extends Http::Server::ResponseHeaderWrite::Range instanceof Http::Server::ResponseHeaderBulkWrite
+ {
+ KeyValuePair item;
+
+ HeaderBulkWriteDictLiteral() {
+ exists(Dict dict | DataFlow::localFlow(DataFlow::exprNode(dict), super.getBulkArg()) |
+ item = dict.getAnItem()
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
+
+ override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
+
+ override predicate nameAllowsNewline() {
+ Http::Server::ResponseHeaderBulkWrite.super.nameAllowsNewline()
+ }
+
+ override predicate valueAllowsNewline() {
+ Http::Server::ResponseHeaderBulkWrite.super.valueAllowsNewline()
+ }
+ }
+
+ /** A tuple in a list for a bulk header update, considered as a single header update. */
+ private class HeaderBulkWriteListLiteral extends Http::Server::ResponseHeaderWrite::Range instanceof Http::Server::ResponseHeaderBulkWrite
+ {
+ Tuple item;
+
+ HeaderBulkWriteListLiteral() {
+ exists(List list | DataFlow::localFlow(DataFlow::exprNode(list), super.getBulkArg()) |
+ item = list.getAnElt()
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result.asExpr() = item.getElt(0) }
+
+ override DataFlow::Node getValueArg() { result.asExpr() = item.getElt(1) }
+
+ override predicate nameAllowsNewline() {
+ Http::Server::ResponseHeaderBulkWrite.super.nameAllowsNewline()
+ }
+
+ override predicate valueAllowsNewline() {
+ Http::Server::ResponseHeaderBulkWrite.super.valueAllowsNewline()
+ }
+ }
+
/**
* A data-flow node that sets a cookie in an HTTP response.
*
diff --git a/python/ql/lib/semmle/python/security/dataflow/HttpHeaderInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/HttpHeaderInjectionCustomizations.qll
index b3fe233629e..e529d3f29e0 100644
--- a/python/ql/lib/semmle/python/security/dataflow/HttpHeaderInjectionCustomizations.qll
+++ b/python/ql/lib/semmle/python/security/dataflow/HttpHeaderInjectionCustomizations.qll
@@ -51,56 +51,6 @@ module HttpHeaderInjection {
}
}
- /** A key-value pair in a literal for a bulk header update, considered as a single header update. */
- // TODO: We could instead consider bulk writes as sinks with an implicit read step of DictionaryKey/DictionaryValue content as needed.
- private class HeaderBulkWriteDictLiteral extends Http::Server::ResponseHeaderWrite::Range instanceof Http::Server::ResponseHeaderBulkWrite
- {
- KeyValuePair item;
-
- HeaderBulkWriteDictLiteral() {
- exists(Dict dict | DataFlow::localFlow(DataFlow::exprNode(dict), super.getBulkArg()) |
- item = dict.getAnItem()
- )
- }
-
- override DataFlow::Node getNameArg() { result.asExpr() = item.getKey() }
-
- override DataFlow::Node getValueArg() { result.asExpr() = item.getValue() }
-
- override predicate nameAllowsNewline() {
- Http::Server::ResponseHeaderBulkWrite.super.nameAllowsNewline()
- }
-
- override predicate valueAllowsNewline() {
- Http::Server::ResponseHeaderBulkWrite.super.valueAllowsNewline()
- }
- }
-
- /** A tuple in a list for a bulk header update, considered as a single header update. */
- // TODO: We could instead consider bulk writes as sinks with implicit read steps as needed.
- private class HeaderBulkWriteListLiteral extends Http::Server::ResponseHeaderWrite::Range instanceof Http::Server::ResponseHeaderBulkWrite
- {
- Tuple item;
-
- HeaderBulkWriteListLiteral() {
- exists(List list | DataFlow::localFlow(DataFlow::exprNode(list), super.getBulkArg()) |
- item = list.getAnElt()
- )
- }
-
- override DataFlow::Node getNameArg() { result.asExpr() = item.getElt(0) }
-
- override DataFlow::Node getValueArg() { result.asExpr() = item.getElt(1) }
-
- override predicate nameAllowsNewline() {
- Http::Server::ResponseHeaderBulkWrite.super.nameAllowsNewline()
- }
-
- override predicate valueAllowsNewline() {
- Http::Server::ResponseHeaderBulkWrite.super.valueAllowsNewline()
- }
- }
-
/**
* A call to replace line breaks, considered as a sanitizer.
*/
From d11f58f768db1bea06bec169d60144f5aa214ec3 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Tue, 28 May 2024 10:47:41 +0100
Subject: [PATCH 21/70] Add cookie header write concept from experimental.
---
python/ql/lib/semmle/python/Concepts.qll | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll
index 8e64deddb4e..c351a7dceed 100644
--- a/python/ql/lib/semmle/python/Concepts.qll
+++ b/python/ql/lib/semmle/python/Concepts.qll
@@ -1234,6 +1234,29 @@ module Http {
}
}
+ /** A write to a `Set-Cookie` header that sets a cookie directly. */
+ private class CookieHeaderWrite extends CookieWrite::Range instanceof Http::Server::ResponseHeaderWrite
+ {
+ CookieHeaderWrite() {
+ exists(StringLiteral str |
+ str.getText() = "Set-Cookie" and
+ DataFlow::exprNode(str)
+ .(DataFlow::LocalSourceNode)
+ .flowsTo(this.(Http::Server::ResponseHeaderWrite).getNameArg())
+ )
+ }
+
+ override DataFlow::Node getNameArg() {
+ result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
+ }
+
+ override DataFlow::Node getHeaderArg() {
+ result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
+ }
+
+ override DataFlow::Node getValueArg() { none() }
+ }
+
/**
* A data-flow node that enables or disables Cross-site request forgery protection
* in a global manner.
From 6b8080a5b3957e88002ef66f7dd3033a57f437a6 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Thu, 6 Jun 2024 15:10:25 +0100
Subject: [PATCH 22/70] Update concept tests for header writes
---
.../test/experimental/meta/ConceptsTest.qll | 33 ++++++++-------
.../frameworks/flask/response_test.py | 42 +++++++++----------
.../stdlib/wsgiref_simple_server_test.py | 10 ++---
3 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/python/ql/test/experimental/meta/ConceptsTest.qll b/python/ql/test/experimental/meta/ConceptsTest.qll
index b552758582b..473c7c177c7 100644
--- a/python/ql/test/experimental/meta/ConceptsTest.qll
+++ b/python/ql/test/experimental/meta/ConceptsTest.qll
@@ -323,8 +323,8 @@ module HttpResponseHeaderWriteTest implements TestSig {
string getARelevantTag() {
result =
[
- "headerWriteNameUnsanitized", "headerWriteNameSanitized", "headerWriteValueUnsanitized",
- "headerWriteValueSanitized", "headerWriteBulk"
+ "headerWriteNameUnsanitized", "headerWriteName", "headerWriteValueUnsanitized",
+ "headerWriteValue", "headerWriteBulk", "headerWriteBulkUnsanitized"
]
}
@@ -339,7 +339,7 @@ module HttpResponseHeaderWriteTest implements TestSig {
(
if write.nameAllowsNewline()
then tag = "headerWriteNameUnsanitized"
- else tag = "headerWriteNameSanitized"
+ else tag = "headerWriteName"
) and
value = prettyNodeForInlineTest(node)
or
@@ -347,7 +347,7 @@ module HttpResponseHeaderWriteTest implements TestSig {
(
if write.valueAllowsNewline()
then tag = "headerWriteValueUnsanitized"
- else tag = "headerWriteValueSanitized"
+ else tag = "headerWriteValue"
) and
value = prettyNodeForInlineTest(node)
)
@@ -360,19 +360,20 @@ module HttpResponseHeaderWriteTest implements TestSig {
tag = "headerWriteBulk" and
value = prettyNodeForInlineTest(node)
or
+ tag = "headerWriteBulkUnsanitized" and
(
- if write.nameAllowsNewline()
- then tag = "headerWriteNameUnsanitized"
- else tag = "headerWriteNameSanitized"
- ) and
- value = ""
- or
- (
- if write.valueAllowsNewline()
- then tag = "headerWriteValueUnsanitized"
- else tag = "headerWriteValueSanitized"
- ) and
- value = ""
+ write.nameAllowsNewline() and
+ not write.valueAllowsNewline() and
+ value = "name"
+ or
+ not write.nameAllowsNewline() and
+ write.valueAllowsNewline() and
+ value = "value"
+ or
+ write.nameAllowsNewline() and
+ write.valueAllowsNewline() and
+ value = "name,value"
+ )
)
)
)
diff --git a/python/ql/test/library-tests/frameworks/flask/response_test.py b/python/ql/test/library-tests/frameworks/flask/response_test.py
index 1359d2f9381..bcfc36ff365 100644
--- a/python/ql/test/library-tests/frameworks/flask/response_test.py
+++ b/python/ql/test/library-tests/frameworks/flask/response_test.py
@@ -118,7 +118,7 @@ def response_modification1(): # $requestHandler
@app.route("/content-type/response-modification2") # $routeSetup="/content-type/response-modification2"
def response_modification2(): # $requestHandler
resp = make_response("hello
") # $HttpResponse mimetype=text/html responseBody="hello
"
- resp.headers["content-type"] = "text/plain" # $ headerWriteNameUnsanitized="content-type" headerWriteValueSanitized="text/plain" MISSING: HttpResponse mimetype=text/plain
+ resp.headers["content-type"] = "text/plain" # $ headerWriteNameUnsanitized="content-type" headerWriteValue="text/plain" MISSING: HttpResponse mimetype=text/plain
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
@@ -148,7 +148,7 @@ def Response3(): # $requestHandler
@app.route("/content-type/Response4") # $routeSetup="/content-type/Response4"
def Response4(): # $requestHandler
# note: capitalization of Content-Type does not matter
- resp = Response("hello
", headers={"Content-TYPE": "text/plain"}) # $ headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized HttpResponse responseBody="hello
" SPURIOUS: mimetype=text/html MISSING: mimetype=text/plain
+ resp = Response("hello
", headers={"Content-TYPE": "text/plain"}) # $ headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="Content-TYPE" headerWriteValue="text/plain" HttpResponse responseBody="hello
" SPURIOUS: mimetype=text/html MISSING: mimetype=text/plain
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
@@ -156,7 +156,7 @@ def Response4(): # $requestHandler
def Response5(): # $requestHandler
# content_type argument takes priority (and result is text/plain)
# note: capitalization of Content-Type does not matter
- resp = Response("hello
", headers={"Content-TYPE": "text/html"}, content_type="text/plain; charset=utf-8") # $ headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized HttpResponse mimetype=text/plain responseBody="hello
"
+ resp = Response("hello
", headers={"Content-TYPE": "text/html"}, content_type="text/plain; charset=utf-8") # $ headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="Content-TYPE" headerWriteValue="text/html" HttpResponse mimetype=text/plain responseBody="hello
"
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
@@ -164,7 +164,7 @@ def Response5(): # $requestHandler
def Response6(): # $requestHandler
# mimetype argument takes priority over header (and result is text/plain)
# note: capitalization of Content-Type does not matter
- resp = Response("hello
", headers={"Content-TYPE": "text/html"}, mimetype="text/plain") # $ headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized HttpResponse mimetype=text/plain responseBody="hello
"
+ resp = Response("hello
", headers={"Content-TYPE": "text/html"}, mimetype="text/plain") # $ headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="Content-TYPE" headerWriteValue="text/html" HttpResponse mimetype=text/plain responseBody="hello
"
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
@@ -208,7 +208,7 @@ def setting_cookie(): # $requestHandler
resp = make_response() # $ HttpResponse mimetype=text/html
resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
- resp.headers.add("Set-Cookie", "key2=value2") # $ headerWriteNameUnsanitized="Set-Cookie" headerWriteValueSanitized="key2=value2" MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers.add("Set-Cookie", "key2=value2") # $ headerWriteNameUnsanitized="Set-Cookie" headerWriteValue="key2=value2" MISSING: CookieWrite CookieRawHeader="key2=value2"
resp.delete_cookie("key3") # $ CookieWrite CookieName="key3"
resp.delete_cookie(key="key3") # $ CookieWrite CookieName="key3"
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
@@ -220,29 +220,29 @@ def setting_cookie(): # $requestHandler
@app.route("/headers") # $routeSetup="/headers"
def headers(): # $requestHandler
resp1 = Response() # $ HttpResponse mimetype=text/html
- resp1.headers["X-MyHeader"] = "a" # $ headerWriteNameUnsanitized="X-MyHeader" headerWriteValueSanitized="a"
+ resp1.headers["X-MyHeader"] = "a" # $ headerWriteNameUnsanitized="X-MyHeader" headerWriteValue="a"
resp2 = make_response() # $ HttpResponse mimetype=text/html
- resp2.headers["X-MyHeader"] = "aa" # $ headerWriteNameUnsanitized="X-MyHeader" headerWriteValueSanitized="aa"
- resp2.headers.extend({"X-MyHeader2": "b"}) # $ headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized
- resp3 = make_response("hello", 200, {"X-MyHeader3": "c"}) # $ HttpResponse mimetype=text/html responseBody="hello" headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized
- resp4 = make_response("hello", {"X-MyHeader4": "d"}) # $ HttpResponse mimetype=text/html responseBody="hello" headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized
- resp5 = Response(headers={"X-MyHeader5":"e"}) # $ HttpResponse mimetype=text/html headerWriteBulk=Dict headerWriteNameUnsanitized headerWriteValueSanitized
+ resp2.headers["X-MyHeader"] = "aa" # $ headerWriteNameUnsanitized="X-MyHeader" headerWriteValue="aa"
+ resp2.headers.extend({"X-MyHeader2": "b"}) # $ headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="X-MyHeader2" headerWriteValue="b"
+ resp3 = make_response("hello", 200, {"X-MyHeader3": "c"}) # $ HttpResponse mimetype=text/html responseBody="hello" headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="X-MyHeader3" headerWriteValue="c"
+ resp4 = make_response("hello", {"X-MyHeader4": "d"}) # $ HttpResponse mimetype=text/html responseBody="hello" headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="X-MyHeader4" headerWriteValue="d"
+ resp5 = Response(headers={"X-MyHeader5":"e"}) # $ HttpResponse mimetype=text/html headerWriteBulk=Dict headerWriteBulkUnsanitized=name headerWriteBulkUnsanitized=name headerWriteNameUnsanitized="X-MyHeader5" headerWriteValue="e"
return resp5 # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp5
@app.route("/werkzeug-headers") # $routeSetup="/werkzeug-headers"
def werkzeug_headers(): # $requestHandler
response = Response() # $ HttpResponse mimetype=text/html
headers = Headers()
- headers.add("X-MyHeader1", "a") # $ headerWriteNameUnsanitized="X-MyHeader1" headerWriteValueSanitized="a"
- headers.add_header("X-MyHeader2", "b") # $ headerWriteNameUnsanitized="X-MyHeader2" headerWriteValueSanitized="b"
- headers.set("X-MyHeader3", "c") # $ headerWriteNameUnsanitized="X-MyHeader3" headerWriteValueSanitized="c"
- headers.setdefault("X-MyHeader4", "d") # $ headerWriteNameUnsanitized="X-MyHeader4" headerWriteValueSanitized="d"
- headers.__setitem__("X-MyHeader5", "e") # $ headerWriteNameUnsanitized="X-MyHeader5" headerWriteValueSanitized="e"
- headers["X-MyHeader6"] = "f" # $ headerWriteNameUnsanitized="X-MyHeader6" headerWriteValueSanitized="f"
- h1 = {"X-MyHeader7": "g"}
- headers.extend(h1) # $ headerWriteBulk=h1 headerWriteNameUnsanitized headerWriteValueSanitized
- h2 = [("X-MyHeader8", "h")]
- headers.extend(h2) # $ headerWriteBulk=h2 headerWriteNameUnsanitized headerWriteValueSanitized
+ headers.add("X-MyHeader1", "a") # $ headerWriteNameUnsanitized="X-MyHeader1" headerWriteValue="a"
+ headers.add_header("X-MyHeader2", "b") # $ headerWriteNameUnsanitized="X-MyHeader2" headerWriteValue="b"
+ headers.set("X-MyHeader3", "c") # $ headerWriteNameUnsanitized="X-MyHeader3" headerWriteValue="c"
+ headers.setdefault("X-MyHeader4", "d") # $ headerWriteNameUnsanitized="X-MyHeader4" headerWriteValue="d"
+ headers.__setitem__("X-MyHeader5", "e") # $ headerWriteNameUnsanitized="X-MyHeader5" headerWriteValue="e"
+ headers["X-MyHeader6"] = "f" # $ headerWriteNameUnsanitized="X-MyHeader6" headerWriteValue="f"
+ h1 = {"X-MyHeader7": "g"} # $ headerWriteNameUnsanitized="X-MyHeader7" headerWriteValue="g"
+ headers.extend(h1) # $ headerWriteBulk=h1 headerWriteBulkUnsanitized=name
+ h2 = [("X-MyHeader8", "h")] # $ headerWriteNameUnsanitized="X-MyHeader8" headerWriteValue="h"
+ headers.extend(h2) # $ headerWriteBulk=h2 headerWriteBulkUnsanitized=name
response.headers = headers
return response # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=response
diff --git a/python/ql/test/library-tests/frameworks/stdlib/wsgiref_simple_server_test.py b/python/ql/test/library-tests/frameworks/stdlib/wsgiref_simple_server_test.py
index 6a2031699f4..7327385c064 100644
--- a/python/ql/test/library-tests/frameworks/stdlib/wsgiref_simple_server_test.py
+++ b/python/ql/test/library-tests/frameworks/stdlib/wsgiref_simple_server_test.py
@@ -18,7 +18,7 @@ def func(environ, start_response): # $ requestHandler
environ, # $ tainted
environ["PATH_INFO"], # $ tainted
)
- write = start_response("200 OK", [("Content-Type", "text/plain")]) # $ headerWriteBulk=List headerWriteNameUnsanitized headerWriteValueUnsanitized
+ write = start_response("200 OK", [("Content-Type", "text/plain")]) # $ headerWriteBulk=List headerWriteBulkUnsanitized=name,value headerWriteNameUnsanitized="Content-Type" headerWriteValueUnsanitized="text/plain"
write(b"hello") # $ HttpResponse responseBody=b"hello"
write(data=b" ") # $ HttpResponse responseBody=b" "
@@ -33,16 +33,16 @@ class MyServer(wsgiref.simple_server.WSGIServer):
self.set_app(self.my_method)
def my_method(self, _env, start_response): # $ requestHandler
- start_response("200 OK", []) # $ headerWriteBulk=List headerWriteNameUnsanitized headerWriteValueUnsanitized
+ start_response("200 OK", []) # $ headerWriteBulk=List headerWriteBulkUnsanitized=name,value
return [b"my_method"] # $ HttpResponse responseBody=List
def func2(environ, start_response): # $ requestHandler
- headers = wsgiref.headers.Headers([("Content-Type", "text/plain")]) # $ headerWriteBulk=List headerWriteNameUnsanitized headerWriteValueUnsanitized
+ headers = wsgiref.headers.Headers([("Content-Type", "text/plain")]) # $ headerWriteBulk=List headerWriteBulkUnsanitized=name,value headerWriteNameUnsanitized="Content-Type" headerWriteValueUnsanitized="text/plain"
headers.add_header("X-MyHeader", "a") # $ headerWriteNameUnsanitized="X-MyHeader" headerWriteValueUnsanitized="a"
headers.setdefault("X-MyHeader2", "b") # $ headerWriteNameUnsanitized="X-MyHeader2" headerWriteValueUnsanitized="b"
headers.__setitem__("X-MyHeader3", "c") # $ headerWriteNameUnsanitized="X-MyHeader3" headerWriteValueUnsanitized="c"
headers["X-MyHeader4"] = "d" # $ headerWriteNameUnsanitized="X-MyHeader4" headerWriteValueUnsanitized="d"
- start_response(status, headers) # $ headerWriteBulk=headers headerWriteNameUnsanitized headerWriteValueUnsanitized
+ start_response(status, headers) # $ headerWriteBulk=headers headerWriteBulkUnsanitized=name,value
return [b"Hello"] # $ HttpResponse responseBody=List
case = sys.argv[1]
@@ -54,7 +54,7 @@ elif case == "2":
elif case == "3":
server = MyServer()
def func3(_env, start_response): # $ requestHandler
- start_response("200 OK", []) # $ headerWriteBulk=List headerWriteNameUnsanitized headerWriteValueUnsanitized
+ start_response("200 OK", []) # $ headerWriteBulk=List headerWriteBulkUnsanitized=name,value
return [b"foo"] # $ HttpResponse responseBody=List
server.set_app(func3)
elif case == "4":
From a0201e9c4ff6e04fad195750d98dc8591ae1d5f6 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Thu, 6 Jun 2024 15:19:02 +0100
Subject: [PATCH 23/70] Update tests for new cookie write from headers
---
python/ql/lib/semmle/python/Concepts.qll | 4 +---
.../ql/test/library-tests/frameworks/flask/response_test.py | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll
index c351a7dceed..74e06b54b0b 100644
--- a/python/ql/lib/semmle/python/Concepts.qll
+++ b/python/ql/lib/semmle/python/Concepts.qll
@@ -1246,9 +1246,7 @@ module Http {
)
}
- override DataFlow::Node getNameArg() {
- result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
- }
+ override DataFlow::Node getNameArg() { none() }
override DataFlow::Node getHeaderArg() {
result = this.(Http::Server::ResponseHeaderWrite).getValueArg()
diff --git a/python/ql/test/library-tests/frameworks/flask/response_test.py b/python/ql/test/library-tests/frameworks/flask/response_test.py
index bcfc36ff365..a1341022c4e 100644
--- a/python/ql/test/library-tests/frameworks/flask/response_test.py
+++ b/python/ql/test/library-tests/frameworks/flask/response_test.py
@@ -208,7 +208,7 @@ def setting_cookie(): # $requestHandler
resp = make_response() # $ HttpResponse mimetype=text/html
resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
- resp.headers.add("Set-Cookie", "key2=value2") # $ headerWriteNameUnsanitized="Set-Cookie" headerWriteValue="key2=value2" MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers.add("Set-Cookie", "key2=value2") # $ headerWriteNameUnsanitized="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
resp.delete_cookie("key3") # $ CookieWrite CookieName="key3"
resp.delete_cookie(key="key3") # $ CookieWrite CookieName="key3"
return resp # $ SPURIOUS: HttpResponse mimetype=text/html responseBody=resp
From 7704801e47a8c10a7597546b51f7e5f17bf1a93f Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Fri, 21 Jun 2024 09:09:14 +0100
Subject: [PATCH 24/70] Change fastapi raw cookie header models to header write
models
---
python/ql/lib/semmle/python/Concepts.qll | 2 +-
.../lib/semmle/python/frameworks/FastApi.qll | 21 +++++++++----------
.../frameworks/fastapi/response_test.py | 8 +++----
3 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll
index 74e06b54b0b..20578e26960 100644
--- a/python/ql/lib/semmle/python/Concepts.qll
+++ b/python/ql/lib/semmle/python/Concepts.qll
@@ -1239,7 +1239,7 @@ module Http {
{
CookieHeaderWrite() {
exists(StringLiteral str |
- str.getText() = "Set-Cookie" and
+ str.getText().toLowerCase() = "set-cookie" and
DataFlow::exprNode(str)
.(DataFlow::LocalSourceNode)
.flowsTo(this.(Http::Server::ResponseHeaderWrite).getNameArg())
diff --git a/python/ql/lib/semmle/python/frameworks/FastApi.qll b/python/ql/lib/semmle/python/frameworks/FastApi.qll
index 8c958e9343d..423f8580a5b 100644
--- a/python/ql/lib/semmle/python/frameworks/FastApi.qll
+++ b/python/ql/lib/semmle/python/frameworks/FastApi.qll
@@ -361,28 +361,27 @@ module FastApi {
}
/**
- * A call to `append` on a `headers` of a FastAPI Response, with the `Set-Cookie`
- * header-key.
+ * A call to `append` on a `headers` of a FastAPI Response.
*/
- private class HeadersAppendCookie extends Http::Server::CookieWrite::Range,
+ private class HeadersAppend extends Http::Server::ResponseHeaderWrite::Range,
DataFlow::MethodCallNode
{
- HeadersAppendCookie() {
- exists(DataFlow::AttrRead headers, DataFlow::Node keyArg |
+ HeadersAppend() {
+ exists(DataFlow::AttrRead headers |
headers.accesses(instance(), "headers") and
- this.calls(headers, "append") and
- keyArg in [this.getArg(0), this.getArgByName("key")] and
- keyArg.getALocalSource().asExpr().(StringLiteral).getText().toLowerCase() = "set-cookie"
+ this.calls(headers, "append")
)
}
- override DataFlow::Node getHeaderArg() {
+ override DataFlow::Node getNameArg() { result = [this.getArg(0), this.getArgByName("key")] }
+
+ override DataFlow::Node getValueArg() {
result in [this.getArg(1), this.getArgByName("value")]
}
- override DataFlow::Node getNameArg() { none() }
+ override predicate nameAllowsNewline() { none() }
- override DataFlow::Node getValueArg() { none() }
+ override predicate valueAllowsNewline() { none() }
}
}
}
diff --git a/python/ql/test/library-tests/frameworks/fastapi/response_test.py b/python/ql/test/library-tests/frameworks/fastapi/response_test.py
index 9f276338c8c..44582d6cd6e 100644
--- a/python/ql/test/library-tests/frameworks/fastapi/response_test.py
+++ b/python/ql/test/library-tests/frameworks/fastapi/response_test.py
@@ -11,9 +11,9 @@ app = FastAPI()
async def response_parameter(response: Response): # $ requestHandler
response.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
response.set_cookie(key="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
- response.headers.append("Set-Cookie", "key2=value2") # $ CookieWrite CookieRawHeader="key2=value2"
- response.headers.append(key="Set-Cookie", value="key2=value2") # $ CookieWrite CookieRawHeader="key2=value2"
- response.headers["X-MyHeader"] = "header-value"
+ response.headers.append("Set-Cookie", "key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
+ response.headers.append(key="Set-Cookie", value="key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
+ response.headers["X-MyHeader"] = "header-value" # $ MISSING: headerWriteName="X-MyHeader" headerWriteValue="header-value"
response.status_code = 418
return {"message": "response as parameter"} # $ HttpResponse mimetype=application/json responseBody=Dict
@@ -45,7 +45,7 @@ async def response_parameter_custom_type(response: MyXmlResponse): # $ requestHa
print(type(response))
assert type(response) == fastapi.responses.Response
response.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
- response.headers["Custom-Response-Type"] = "yes, but only after function has run"
+ response.headers["Custom-Response-Type"] = "yes, but only after function has run" # $ MISSING: headerWriteName="Custom-Response-Typer" headerWriteValue="yes, but only after function has run"
xml_data = "FOO"
return xml_data # $ HttpResponse responseBody=xml_data mimetype=application/xml
From 5ced5c010c9450a6d393652e60c3e7ffe97abddd Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Fri, 21 Jun 2024 11:29:20 +0100
Subject: [PATCH 25/70] Add django header writes
---
.../lib/semmle/python/frameworks/Django.qll | 31 +++++++++++++++++++
.../frameworks/django-v2-v3/response_test.py | 4 +--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll
index 064dba57f92..7c0befa6349 100644
--- a/python/ql/lib/semmle/python/frameworks/Django.qll
+++ b/python/ql/lib/semmle/python/frameworks/Django.qll
@@ -2239,6 +2239,37 @@ module PrivateDjango {
override DataFlow::Node getValueArg() { result = value }
}
+
+ class DjangoResponseHeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
+ DataFlow::Node index;
+ DataFlow::Node value;
+
+ DjangoResponseHeaderSubscriptWrite() {
+ exists(SubscriptNode subscript, DataFlow::AttrRead headerLookup |
+ // To give `this` a value, we need to choose between either LHS or RHS,
+ // and just go with the LHS
+ this.asCfgNode() = subscript
+ |
+ headerLookup
+ .accesses(DjangoImpl::DjangoHttp::Response::HttpResponse::instance(), "headers") and
+ exists(DataFlow::Node subscriptObj |
+ subscriptObj.asCfgNode() = subscript.getObject()
+ |
+ headerLookup.flowsTo(subscriptObj)
+ ) and
+ value.asCfgNode() = subscript.(DefinitionNode).getValue() and
+ index.asCfgNode() = subscript.getIndex()
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result = index }
+
+ override DataFlow::Node getValueArg() { result = value }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
}
diff --git a/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py b/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
index dd78cd51016..d4f17f7c3cf 100644
--- a/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
+++ b/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
@@ -72,7 +72,7 @@ def redirect_through_normal_response_new_headers_attr(request):
resp = HttpResponse() # $ HttpResponse mimetype=text/html
resp.status_code = 302
- resp.headers['Location'] = next # $ MISSING: redirectLocation=next
+ resp.headers['Location'] = next # $ headerWriteName='Location' headerWriteValue=next MISSING: redirectLocation=next
resp.content = private # $ MISSING: responseBody=private
return resp
@@ -130,7 +130,7 @@ def setting_cookie(request):
resp = HttpResponse() # $ HttpResponse mimetype=text/html
resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
- resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers["Set-Cookie"] = "key2=value2" # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3"
resp.delete_cookie("key4") # $ CookieWrite CookieName="key4"
resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4"
From 79c0ed607487f7c59d903113f576dfdfde60bcca Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Fri, 21 Jun 2024 14:01:33 +0100
Subject: [PATCH 26/70] Add additional fastapi mheader write models
---
.../lib/semmle/python/frameworks/FastApi.qll | 28 +++++++++++++++++++
.../frameworks/fastapi/response_test.py | 4 +--
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/python/ql/lib/semmle/python/frameworks/FastApi.qll b/python/ql/lib/semmle/python/frameworks/FastApi.qll
index 423f8580a5b..028c5f26601 100644
--- a/python/ql/lib/semmle/python/frameworks/FastApi.qll
+++ b/python/ql/lib/semmle/python/frameworks/FastApi.qll
@@ -383,5 +383,33 @@ module FastApi {
override predicate valueAllowsNewline() { none() }
}
+
+ class HeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
+ DataFlow::Node index;
+ DataFlow::Node value;
+
+ HeaderSubscriptWrite() {
+ exists(SubscriptNode subscript, DataFlow::AttrRead headerLookup |
+ // To give `this` a value, we need to choose between either LHS or RHS,
+ // and just go with the LHS
+ this.asCfgNode() = subscript
+ |
+ headerLookup.accesses(instance(), "headers") and
+ exists(DataFlow::Node subscriptObj | subscriptObj.asCfgNode() = subscript.getObject() |
+ headerLookup.flowsTo(subscriptObj)
+ ) and
+ value.asCfgNode() = subscript.(DefinitionNode).getValue() and
+ index.asCfgNode() = subscript.getIndex()
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result = index }
+
+ override DataFlow::Node getValueArg() { result = value }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
}
diff --git a/python/ql/test/library-tests/frameworks/fastapi/response_test.py b/python/ql/test/library-tests/frameworks/fastapi/response_test.py
index 44582d6cd6e..2bc26c15fda 100644
--- a/python/ql/test/library-tests/frameworks/fastapi/response_test.py
+++ b/python/ql/test/library-tests/frameworks/fastapi/response_test.py
@@ -13,7 +13,7 @@ async def response_parameter(response: Response): # $ requestHandler
response.set_cookie(key="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
response.headers.append("Set-Cookie", "key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
response.headers.append(key="Set-Cookie", value="key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
- response.headers["X-MyHeader"] = "header-value" # $ MISSING: headerWriteName="X-MyHeader" headerWriteValue="header-value"
+ response.headers["X-MyHeader"] = "header-value" # $ headerWriteName="X-MyHeader" headerWriteValue="header-value"
response.status_code = 418
return {"message": "response as parameter"} # $ HttpResponse mimetype=application/json responseBody=Dict
@@ -45,7 +45,7 @@ async def response_parameter_custom_type(response: MyXmlResponse): # $ requestHa
print(type(response))
assert type(response) == fastapi.responses.Response
response.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
- response.headers["Custom-Response-Type"] = "yes, but only after function has run" # $ MISSING: headerWriteName="Custom-Response-Typer" headerWriteValue="yes, but only after function has run"
+ response.headers["Custom-Response-Type"] = "yes, but only after function has run" # $ headerWriteName="Custom-Response-Type" headerWriteValue="yes, but only after function has run"
xml_data = "FOO"
return xml_data # $ HttpResponse responseBody=xml_data mimetype=application/xml
From c404f00a9b228366393a2bf15939b4cf76a10a7f Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Mon, 24 Jun 2024 10:41:07 +0100
Subject: [PATCH 27/70] Add additional header write models for aiohttp and
tornado + added qldoc
---
.../lib/semmle/python/frameworks/Aiohttp.qll | 27 ++++++++
.../lib/semmle/python/frameworks/Django.qll | 4 ++
.../lib/semmle/python/frameworks/FastApi.qll | 4 ++
.../lib/semmle/python/frameworks/Tornado.qll | 63 +++++++++++++++++++
.../frameworks/aiohttp/response_test.py | 2 +-
.../frameworks/tornado/response_test.py | 9 ++-
6 files changed, 105 insertions(+), 4 deletions(-)
diff --git a/python/ql/lib/semmle/python/frameworks/Aiohttp.qll b/python/ql/lib/semmle/python/frameworks/Aiohttp.qll
index 78d269c31d3..517b309594a 100644
--- a/python/ql/lib/semmle/python/frameworks/Aiohttp.qll
+++ b/python/ql/lib/semmle/python/frameworks/Aiohttp.qll
@@ -706,6 +706,33 @@ module AiohttpWebModel {
override DataFlow::Node getValueArg() { result = value }
}
+
+ /**
+ * A dict-like write to an item of the `headers` attribute on a HTTP response, such as
+ * `response.headers[name] = value`.
+ */
+ class AiohttpResponseHeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
+ DataFlow::Node index;
+ DataFlow::Node value;
+
+ AiohttpResponseHeaderSubscriptWrite() {
+ exists(API::Node i |
+ value = aiohttpResponseInstance().getMember("headers").getSubscriptAt(i).asSink() and
+ index = i.asSink() and
+ // To give `this` a value, we need to choose between either LHS or RHS,
+ // and just go with the RHS as it is readily available
+ this = value
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result = index }
+
+ override DataFlow::Node getValueArg() { result = value }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
/**
diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll
index 7c0befa6349..69b0e8710da 100644
--- a/python/ql/lib/semmle/python/frameworks/Django.qll
+++ b/python/ql/lib/semmle/python/frameworks/Django.qll
@@ -2240,6 +2240,10 @@ module PrivateDjango {
override DataFlow::Node getValueArg() { result = value }
}
+ /**
+ * A dict-like write to an item of the `headers` attribute on a HTTP response, such as
+ * `response.headers[name] = value`.
+ */
class DjangoResponseHeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
DataFlow::Node index;
DataFlow::Node value;
diff --git a/python/ql/lib/semmle/python/frameworks/FastApi.qll b/python/ql/lib/semmle/python/frameworks/FastApi.qll
index 028c5f26601..0793b4b7d6a 100644
--- a/python/ql/lib/semmle/python/frameworks/FastApi.qll
+++ b/python/ql/lib/semmle/python/frameworks/FastApi.qll
@@ -384,6 +384,10 @@ module FastApi {
override predicate valueAllowsNewline() { none() }
}
+ /**
+ * A dict-like write to an item of the `headers` attribute on a HTTP response, such as
+ * `response.headers[name] = value`.
+ */
class HeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
DataFlow::Node index;
DataFlow::Node value;
diff --git a/python/ql/lib/semmle/python/frameworks/Tornado.qll b/python/ql/lib/semmle/python/frameworks/Tornado.qll
index 1bd40603296..7bc4cf25794 100644
--- a/python/ql/lib/semmle/python/frameworks/Tornado.qll
+++ b/python/ql/lib/semmle/python/frameworks/Tornado.qll
@@ -63,6 +63,50 @@ module Tornado {
override string getAsyncMethodName() { none() }
}
+
+ /**
+ * A dict-like write to an item of an `HTTPHeaders` object.
+ */
+ private class TornadoHeaderSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
+ DataFlow::Node index;
+ DataFlow::Node value;
+
+ TornadoHeaderSubscriptWrite() {
+ exists(SubscriptNode subscript |
+ subscript.getObject() = instance().asCfgNode() and
+ value.asCfgNode() = subscript.(DefinitionNode).getValue() and
+ index.asCfgNode() = subscript.getIndex() and
+ this.asCfgNode() = subscript
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result = index }
+
+ override DataFlow::Node getValueArg() { result = value }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
+
+ /**
+ * A call to `HTTPHeaders.add`.
+ */
+ private class TornadoHeadersAppendCall extends Http::Server::ResponseHeaderWrite::Range,
+ DataFlow::MethodCallNode
+ {
+ TornadoHeadersAppendCall() { this.calls(instance(), "append") }
+
+ override DataFlow::Node getNameArg() { result = [this.getArg(0), this.getArgByName("name")] }
+
+ override DataFlow::Node getValueArg() {
+ result in [this.getArg(1), this.getArgByName("value")]
+ }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
// ---------------------------------------------------------------------------
@@ -209,6 +253,25 @@ module Tornado {
this.(DataFlow::AttrRead).getAttributeName() = "request"
}
}
+
+ /** A call to `RequestHandler.set_header` or `RequestHandler.add_header` */
+ private class TornadoSetHeaderCall extends Http::Server::ResponseHeaderWrite::Range,
+ DataFlow::MethodCallNode
+ {
+ TornadoSetHeaderCall() { this.calls(instance(), ["set_header", "add_header"]) }
+
+ override DataFlow::Node getNameArg() {
+ result = [this.getArg(0), this.getArgByName("name")]
+ }
+
+ override DataFlow::Node getValueArg() {
+ result in [this.getArg(1), this.getArgByName("value")]
+ }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
/**
diff --git a/python/ql/test/library-tests/frameworks/aiohttp/response_test.py b/python/ql/test/library-tests/frameworks/aiohttp/response_test.py
index bc9bc8d3bda..a40bf0bbc65 100644
--- a/python/ql/test/library-tests/frameworks/aiohttp/response_test.py
+++ b/python/ql/test/library-tests/frameworks/aiohttp/response_test.py
@@ -96,7 +96,7 @@ async def streaming_response(request): # $ requestHandler
async def setting_cookie(request): # $ requestHandler
resp = web.Response(text="foo") # $ HttpResponse mimetype=text/plain responseBody="foo"
resp.cookies["key"] = "value" # $ CookieWrite CookieName="key" CookieValue="value"
- resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers["Set-Cookie"] = "key2=value2" # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
resp.set_cookie("key3", "value3") # $ CookieWrite CookieName="key3" CookieValue="value3"
resp.set_cookie(name="key3", value="value3") # $ CookieWrite CookieName="key3" CookieValue="value3"
resp.del_cookie("key4") # $ CookieWrite CookieName="key4"
diff --git a/python/ql/test/library-tests/frameworks/tornado/response_test.py b/python/ql/test/library-tests/frameworks/tornado/response_test.py
index 1ca37ed773c..1685ac4d158 100644
--- a/python/ql/test/library-tests/frameworks/tornado/response_test.py
+++ b/python/ql/test/library-tests/frameworks/tornado/response_test.py
@@ -24,10 +24,10 @@ class ExplicitContentType(tornado.web.RequestHandler):
# what matters.
self.write("foo") # $ HttpResponse mimetype=text/html responseBody="foo"
- self.set_header("Content-Type", "text/plain; charset=utf-8")
+ self.set_header("Content-Type", "text/plain; charset=utf-8") # $ headerWriteName="Content-Type" headerWriteValue="text/plain; charset=utf-8"
def post(self): # $ requestHandler
- self.set_header("Content-Type", "text/plain; charset=utf-8")
+ self.set_header("Content-Type", "text/plain; charset=utf-8") # $ headerWriteName="Content-Type" headerWriteValue="text/plain; charset=utf-8"
self.write("foo") # $ HttpResponse responseBody="foo" MISSING: mimetype=text/plain SPURIOUS: mimetype=text/html
@@ -67,7 +67,10 @@ class CookieWriting(tornado.web.RequestHandler):
self.write("foo") # $ HttpResponse mimetype=text/html responseBody="foo"
self.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
self.set_cookie(name="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
- self.set_header("Set-Cookie", "key2=value2") # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
+ self.set_header("Set-Cookie", "key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
+ self.add_header("Set-Cookie", "key3=value3") # $ headerWriteName="Set-Cookie" headerWriteValue="key3=value3" CookieWrite CookieRawHeader="key3=value3"
+ self.request.headers.append("Set-Cookie", "key4=value4") # $ headerWriteName="Set-Cookie" headerWriteValue="key4=value4" CookieWrite CookieRawHeader="key4=value4"
+ self.request.headers["Set-Cookie"] = "key5=value5" # $ headerWriteName="Set-Cookie" headerWriteValue="key5=value5" CookieWrite CookieRawHeader="key5=value5"
def make_app():
From d0f735ac28c9747ff2bc7a6f20a5092daf667da4 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Mon, 24 Jun 2024 20:52:09 +0100
Subject: [PATCH 28/70] Update tests for restframework
---
.../library-tests/frameworks/rest_framework/response_test.py | 2 +-
.../library-tests/frameworks/rest_framework/testapp/views.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/ql/test/library-tests/frameworks/rest_framework/response_test.py b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py
index ec093499df6..3e4f821693b 100644
--- a/python/ql/test/library-tests/frameworks/rest_framework/response_test.py
+++ b/python/ql/test/library-tests/frameworks/rest_framework/response_test.py
@@ -28,7 +28,7 @@ def setting_cookie(request):
resp = Response() # $ HttpResponse
resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key4", value="value") # $ CookieWrite CookieName="key4" CookieValue="value"
- resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers["Set-Cookie"] = "key2=value2" # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3"
resp.delete_cookie("key4") # $ CookieWrite CookieName="key4"
resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4"
diff --git a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py
index 6affb5dac4b..6ce06fdba31 100644
--- a/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py
+++ b/python/ql/test/library-tests/frameworks/rest_framework/testapp/views.py
@@ -70,7 +70,7 @@ def cookie_test(request: Request): # $ requestHandler
resp = Response("wat") # $ HttpResponse
resp.set_cookie("key", "value") # $ CookieWrite CookieName="key" CookieValue="value"
resp.set_cookie(key="key4", value="value") # $ CookieWrite CookieName="key4" CookieValue="value"
- resp.headers["Set-Cookie"] = "key2=value2" # $ MISSING: CookieWrite CookieRawHeader="key2=value2"
+ resp.headers["Set-Cookie"] = "key2=value2" # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3"
return resp
From 0901b3d0a67aa4836161c9d188b706f38d5a1346 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Mon, 24 Jun 2024 21:43:09 +0100
Subject: [PATCH 29/70] Add change note
---
python/ql/lib/change-notes/2024-06-24-cookie-header-writes.md | 4 ++++
1 file changed, 4 insertions(+)
create mode 100644 python/ql/lib/change-notes/2024-06-24-cookie-header-writes.md
diff --git a/python/ql/lib/change-notes/2024-06-24-cookie-header-writes.md b/python/ql/lib/change-notes/2024-06-24-cookie-header-writes.md
new file mode 100644
index 00000000000..583e0f44c05
--- /dev/null
+++ b/python/ql/lib/change-notes/2024-06-24-cookie-header-writes.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Additional modelling has been added to detect cookie writes from direct writes to the `Set-Cookie` header have been added for several web frameworks.
\ No newline at end of file
From 6538d22d3f32dce3a8762bbe15b1621e3c34a556 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Wed, 26 Jun 2024 09:21:53 +0100
Subject: [PATCH 30/70] Fix tornado model of httheaders.add.
---
python/ql/lib/semmle/python/frameworks/Tornado.qll | 2 +-
.../ql/test/library-tests/frameworks/tornado/response_test.py | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/ql/lib/semmle/python/frameworks/Tornado.qll b/python/ql/lib/semmle/python/frameworks/Tornado.qll
index 7bc4cf25794..214f2fb9bad 100644
--- a/python/ql/lib/semmle/python/frameworks/Tornado.qll
+++ b/python/ql/lib/semmle/python/frameworks/Tornado.qll
@@ -95,7 +95,7 @@ module Tornado {
private class TornadoHeadersAppendCall extends Http::Server::ResponseHeaderWrite::Range,
DataFlow::MethodCallNode
{
- TornadoHeadersAppendCall() { this.calls(instance(), "append") }
+ TornadoHeadersAppendCall() { this.calls(instance(), "add") }
override DataFlow::Node getNameArg() { result = [this.getArg(0), this.getArgByName("name")] }
diff --git a/python/ql/test/library-tests/frameworks/tornado/response_test.py b/python/ql/test/library-tests/frameworks/tornado/response_test.py
index 1685ac4d158..a1054f28dc9 100644
--- a/python/ql/test/library-tests/frameworks/tornado/response_test.py
+++ b/python/ql/test/library-tests/frameworks/tornado/response_test.py
@@ -69,7 +69,7 @@ class CookieWriting(tornado.web.RequestHandler):
self.set_cookie(name="key", value="value") # $ CookieWrite CookieName="key" CookieValue="value"
self.set_header("Set-Cookie", "key2=value2") # $ headerWriteName="Set-Cookie" headerWriteValue="key2=value2" CookieWrite CookieRawHeader="key2=value2"
self.add_header("Set-Cookie", "key3=value3") # $ headerWriteName="Set-Cookie" headerWriteValue="key3=value3" CookieWrite CookieRawHeader="key3=value3"
- self.request.headers.append("Set-Cookie", "key4=value4") # $ headerWriteName="Set-Cookie" headerWriteValue="key4=value4" CookieWrite CookieRawHeader="key4=value4"
+ self.request.headers.add("Set-Cookie", "key4=value4") # $ headerWriteName="Set-Cookie" headerWriteValue="key4=value4" CookieWrite CookieRawHeader="key4=value4"
self.request.headers["Set-Cookie"] = "key5=value5" # $ headerWriteName="Set-Cookie" headerWriteValue="key5=value5" CookieWrite CookieRawHeader="key5=value5"
From f22778960bfdaa674adff0d1fac1392350048f93 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Wed, 26 Jun 2024 11:31:57 +0100
Subject: [PATCH 31/70] Fixed expected test results for Helmet query
---
.../test/query-tests/Security/CWE-693/InsecureHelmet.expected | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.expected b/javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.expected
index 7368d96f3d4..2c9407136aa 100644
--- a/javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-693/InsecureHelmet.expected
@@ -1,2 +1,2 @@
-| InsecureHelmetBad.js:7:5:7:32 | content ... : false | Helmet route handler, called with $@ set to 'false' | InsecureHelmetBad.js:7:5:7:32 | content ... : false | contentSecurityPolicy |
-| InsecureHelmetBad.js:8:5:8:21 | frameguard: false | Helmet route handler, called with $@ set to 'false' | InsecureHelmetBad.js:8:5:8:21 | frameguard: false | frameguard |
+| 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 |
From b81d41ba7b64c81717ff49ce51be293ff344a0e5 Mon Sep 17 00:00:00 2001
From: Joe Farebrother
Date: Mon, 1 Jul 2024 11:26:54 +0100
Subject: [PATCH 32/70] Add django header write models for direct subscript
write
---
.../lib/semmle/python/frameworks/Django.qll | 30 +++++++++++++++++++
.../Security/CWE-614/CookieInjection.expected | 11 +++++++
.../Security/CWE-614/InsecureCookie.expected | 6 ++++
.../Security/CWE-614/django_bad.py | 4 +--
.../frameworks/django-v2-v3/response_test.py | 3 +-
5 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/python/ql/lib/semmle/python/frameworks/Django.qll b/python/ql/lib/semmle/python/frameworks/Django.qll
index 69b0e8710da..b3b027e48ff 100644
--- a/python/ql/lib/semmle/python/frameworks/Django.qll
+++ b/python/ql/lib/semmle/python/frameworks/Django.qll
@@ -2274,6 +2274,36 @@ module PrivateDjango {
override predicate valueAllowsNewline() { none() }
}
+
+ /**
+ * A dict-like write to an item of an HTTP response, which is treated as a header write,
+ * such as `response[headerName] = value`
+ */
+ class DjangoResponseSubscriptWrite extends Http::Server::ResponseHeaderWrite::Range {
+ DataFlow::Node index;
+ DataFlow::Node value;
+
+ DjangoResponseSubscriptWrite() {
+ exists(SubscriptNode subscript |
+ // To give `this` a value, we need to choose between either LHS or RHS,
+ // and just go with the LHS
+ this.asCfgNode() = subscript
+ |
+ subscript.getObject() =
+ DjangoImpl::DjangoHttp::Response::HttpResponse::instance().asCfgNode() and
+ value.asCfgNode() = subscript.(DefinitionNode).getValue() and
+ index.asCfgNode() = subscript.getIndex()
+ )
+ }
+
+ override DataFlow::Node getNameArg() { result = index }
+
+ override DataFlow::Node getValueArg() { result = value }
+
+ override predicate nameAllowsNewline() { none() }
+
+ override predicate valueAllowsNewline() { none() }
+ }
}
}
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected b/python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected
index 1b3120c8546..80dcbae2177 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-614/CookieInjection.expected
@@ -1,4 +1,6 @@
edges
+| django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | provenance | |
+| django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | provenance | |
| flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:1:26:1:32 | ControlFlowNode for request | provenance | |
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | flask_bad.py:24:21:24:27 | ControlFlowNode for request | provenance | |
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | flask_bad.py:24:49:24:55 | ControlFlowNode for request | provenance | |
@@ -12,6 +14,9 @@ edges
nodes
| django_bad.py:19:21:19:55 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | semmle.label | ControlFlowNode for Fstring |
+| django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
+| django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | semmle.label | ControlFlowNode for Attribute() |
| flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | semmle.label | ControlFlowNode for ImportMember |
| flask_bad.py:1:26:1:32 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
| flask_bad.py:24:21:24:27 | ControlFlowNode for request | semmle.label | ControlFlowNode for request |
@@ -29,6 +34,12 @@ subpaths
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
| django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:20:21:20:56 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:27:33:27:67 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its httponly flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its samesite flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
+| django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | django_bad.py:27:30:27:124 | ControlFlowNode for Fstring | Cookie is constructed from a $@,and its secure flag is not properly set. | django_bad.py:27:71:27:106 | ControlFlowNode for Attribute() | user-supplied input |
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its httponly flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its samesite flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |
| flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | flask_bad.py:24:21:24:40 | ControlFlowNode for Subscript | Cookie is constructed from a $@,and its secure flag is not properly set. | flask_bad.py:1:26:1:32 | ControlFlowNode for ImportMember | user-supplied input |
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected b/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected
index 2743a8d2116..61f9b9b7469 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected
+++ b/python/ql/test/experimental/query-tests/Security/CWE-614/InsecureCookie.expected
@@ -1,9 +1,15 @@
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
| django_bad.py:6:5:7:52 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |
+| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. |
+| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. |
+| django_bad.py:13:5:13:26 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. |
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
| django_bad.py:19:5:21:66 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |
+| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'httponly' flag properly set. |
+| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'samesite' flag properly set. |
+| django_bad.py:27:5:27:26 | ControlFlowNode for Subscript | Cookie is added without the 'secure' flag properly set. |
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'httponly' flag properly set. |
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'samesite' flag properly set. |
| django_good.py:19:5:19:44 | ControlFlowNode for Attribute() | Cookie is added without the 'secure' flag properly set. |
diff --git a/python/ql/test/experimental/query-tests/Security/CWE-614/django_bad.py b/python/ql/test/experimental/query-tests/Security/CWE-614/django_bad.py
index 45cece4390f..6f1916930e5 100644
--- a/python/ql/test/experimental/query-tests/Security/CWE-614/django_bad.py
+++ b/python/ql/test/experimental/query-tests/Security/CWE-614/django_bad.py
@@ -7,7 +7,7 @@ def django_response(request):
httponly=False, samesite='None')
return resp
-# This test no longer produces an output due to django header setting methods not being modeled in the main query pack
+
def django_response():
response = django.http.HttpResponse()
response['Set-Cookie'] = "name=value; SameSite=None;"
@@ -21,7 +21,7 @@ def django_response(request):
secure=False, httponly=False, samesite='None')
return resp
-# This test no longer produces an output due to django header setting methods not being modeled in the main query pack
+
def django_response():
response = django.http.HttpResponse()
response['Set-Cookie'] = f"{django.http.request.GET.get('name')}={django.http.request.GET.get('value')}; SameSite=None;"
diff --git a/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py b/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
index d4f17f7c3cf..7722b4de8e1 100644
--- a/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
+++ b/python/ql/test/library-tests/frameworks/django-v2-v3/response_test.py
@@ -62,7 +62,7 @@ def redirect_through_normal_response(request):
resp = HttpResponse() # $ HttpResponse mimetype=text/html
resp.status_code = 302
- resp['Location'] = next # $ MISSING: redirectLocation=next
+ resp['Location'] = next # $ headerWriteName='Location' headerWriteValue=next MISSING: redirectLocation=next
resp.content = private # $ MISSING: responseBody=private
return resp
@@ -134,4 +134,5 @@ def setting_cookie(request):
resp.cookies["key3"] = "value3" # $ CookieWrite CookieName="key3" CookieValue="value3"
resp.delete_cookie("key4") # $ CookieWrite CookieName="key4"
resp.delete_cookie(key="key4") # $ CookieWrite CookieName="key4"
+ resp["Set-Cookie"] = "key5=value5" # $ headerWriteName="Set-Cookie" headerWriteValue="key5=value5" CookieWrite CookieRawHeader="key5=value5"
return resp
From d1d082982ac5f63ed0a60f4de41ec748dd38dfe8 Mon Sep 17 00:00:00 2001
From: aegilops <41705651+aegilops@users.noreply.github.com>
Date: Mon, 1 Jul 2024 14:25:29 +0100
Subject: [PATCH 33/70] More external references
---
javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
index e294779d6b8..30fb2f89179 100644
--- a/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
+++ b/javascript/ql/src/Security/CWE-693/InsecureHelmet.qhelp
@@ -57,5 +57,14 @@
helmet.js website
+
+ Content Security Policy (CSP) | MDN
+
+
+ Mozilla Web Security Guidelines
+
+
+ Protect against clickjacking | MDN
+