From c14faa3c86bb93d081f5b8daa2ef70eb56a2f38a Mon Sep 17 00:00:00 2001
From: Rob Hudson <robhudson@mozilla.com>
Date: Fri, 3 Jan 2025 16:19:40 -0800
Subject: [PATCH] Fix #249: Add `EXCLUDE_URL_PREFIXES` check

---
 csp/apps.py              |  6 +-----
 csp/checks.py            | 38 +++++++++++++++++++++++++++++++++++++-
 csp/tests/test_checks.py | 24 +++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/csp/apps.py b/csp/apps.py
index 2013782..3a7c43a 100644
--- a/csp/apps.py
+++ b/csp/apps.py
@@ -1,11 +1,7 @@
 from django.apps import AppConfig
-from django.core import checks
 
-from csp.checks import check_django_csp_lt_4_0
+from csp.checks import *  # noqa: F403 (here to register the checks)
 
 
 class CspConfig(AppConfig):
     name = "csp"
-
-    def ready(self) -> None:
-        checks.register(check_django_csp_lt_4_0, checks.Tags.security)
diff --git a/csp/checks.py b/csp/checks.py
index 3eba135..38cf95e 100644
--- a/csp/checks.py
+++ b/csp/checks.py
@@ -2,10 +2,14 @@
 
 import pprint
 from collections.abc import Sequence
+from importlib.metadata import version
 from typing import TYPE_CHECKING, Any
 
 from django.conf import settings
-from django.core.checks import Error
+from django.core import checks
+from django.core.checks import Error, register
+
+from packaging.version import Version
 
 from csp.constants import NONCE
 
@@ -77,6 +81,7 @@ def migrate_settings() -> tuple[dict[str, Any], bool]:
     return config, REPORT_ONLY
 
 
+@register(checks.Tags.security)
 def check_django_csp_lt_4_0(app_configs: Sequence[AppConfig] | None, **kwargs: Any) -> list[Error]:
     check_settings = OUTDATED_SETTINGS + ["CSP_REPORT_ONLY", "CSP_EXCLUDE_URL_PREFIXES", "CSP_REPORT_PERCENTAGE"]
     if any(hasattr(settings, setting) for setting in check_settings):
@@ -91,3 +96,34 @@ def check_django_csp_lt_4_0(app_configs: Sequence[AppConfig] | None, **kwargs: A
         return [Error(warning, id="csp.E001")]
 
     return []
+
+
+@register(checks.Tags.security)
+def check_exclude_url_prefixes_is_not_string(app_configs: Sequence[AppConfig] | None, **kwargs: Any) -> list[Error]:
+    """
+    Check that EXCLUDE_URL_PREFIXES in settings is not a string.
+
+    If it is a string it can lead to a security issue where the string is treated as a list of
+    characters, resulting in '/' matching all paths excluding the CSP header from all responses.
+
+    """
+    # Skip check for django-csp < 4.0.
+    if Version(version("django-csp")) < Version("4.0a1"):
+        return []
+
+    errors = []
+    keys = (
+        "CONTENT_SECURITY_POLICY",
+        "CONTENT_SECURITY_POLICY_REPORT_ONLY",
+    )
+    for key in keys:
+        config = getattr(settings, key, {})
+        if isinstance(config, dict) and isinstance(config.get("EXCLUDE_URL_PREFIXES"), str):
+            errors.append(
+                Error(
+                    f"EXCLUDE_URL_PREFIXES in {key} settings must be a list or tuple.",
+                    id="csp.E002",
+                )
+            )
+
+    return errors
diff --git a/csp/tests/test_checks.py b/csp/tests/test_checks.py
index c4a422b..7ec3865 100644
--- a/csp/tests/test_checks.py
+++ b/csp/tests/test_checks.py
@@ -1,6 +1,6 @@
 from django.test.utils import override_settings
 
-from csp.checks import check_django_csp_lt_4_0, migrate_settings
+from csp.checks import check_django_csp_lt_4_0, check_exclude_url_prefixes_is_not_string, migrate_settings
 from csp.constants import NONCE
 
 
@@ -50,3 +50,25 @@ def test_check_django_csp_lt_4_0() -> None:
 
 def test_check_django_csp_lt_4_0_no_config() -> None:
     assert check_django_csp_lt_4_0(None) == []
+
+
+@override_settings(
+    CONTENT_SECURITY_POLICY={"EXCLUDE_URL_PREFIXES": "/admin/"},
+)
+def test_check_exclude_url_prefixes_is_not_string() -> None:
+    errors = check_exclude_url_prefixes_is_not_string(None)
+    assert len(errors) == 1
+    error = errors[0]
+    assert error.id == "csp.E002"
+    assert error.msg == "EXCLUDE_URL_PREFIXES in CONTENT_SECURITY_POLICY settings must be a list or tuple."
+
+
+@override_settings(
+    CONTENT_SECURITY_POLICY_REPORT_ONLY={"EXCLUDE_URL_PREFIXES": "/admin/"},
+)
+def test_check_exclude_url_prefixes_ro_is_not_string() -> None:
+    errors = check_exclude_url_prefixes_is_not_string(None)
+    assert len(errors) == 1
+    error = errors[0]
+    assert error.id == "csp.E002"
+    assert error.msg == "EXCLUDE_URL_PREFIXES in CONTENT_SECURITY_POLICY_REPORT_ONLY settings must be a list or tuple."