Fixing GHSA-vwgh-2hvh-4xm5 — substring match in the shared_diff_access

changedetectionio/auth_decorator.py

@@ -3,6 +3,16 @@ from functools import wraps
 from flask import current_app, redirect, request
 from loguru import logger
 
+# Endpoints exempt from auth when `shared_diff_access` is enabled.
+# Must be exact endpoint names — substring matching (GHSA-vwgh-2hvh-4xm5)
+# let the state-changing `/diff/<uuid>/extract` endpoints slip through
+# because their names share the `diff_history_page` prefix.
+SHARED_DIFF_READ_ONLY_ENDPOINTS = frozenset({
+    'ui.ui_diff.diff_history_page',
+    'ui.ui_diff.processor_asset',
+    'ui.ui_diff.download_patch',
+})
+
 def login_optionally_required(func):
     """
     If password authentication is enabled, verify the user is logged in.
@@ -20,7 +30,7 @@ def login_optionally_required(func):
         has_password_enabled = datastore.data['settings']['application'].get('password') or os.getenv("SALTED_PASS", False)
 
         # Permitted
-        if request.endpoint and 'diff_history_page' in request.endpoint and datastore.data['settings']['application'].get('shared_diff_access'):
+        if request.endpoint in SHARED_DIFF_READ_ONLY_ENDPOINTS and datastore.data['settings']['application'].get('shared_diff_access'):
             return func(*args, **kwargs)
         elif request.method in flask_login.config.EXEMPT_METHODS:
             return func(*args, **kwargs)

changedetectionio/flask_app.py

@@ -414,7 +414,7 @@ def _jinja2_filter_sanitize_tag_class(tag_title):
     return sanitized if sanitized else 'tag'
 
 # Import login_optionally_required from auth_decorator
-from changedetectionio.auth_decorator import login_optionally_required
+from changedetectionio.auth_decorator import SHARED_DIFF_READ_ONLY_ENDPOINTS, login_optionally_required
 
 # When nobody is logged in Flask-Login's current_user is set to an AnonymousUser object.
 class User(flask_login.UserMixin):
@@ -541,7 +541,7 @@ def changedetection_app(config=None, datastore_o=None):
             # Permitted
             elif request.endpoint and 'login' in request.endpoint:
                 return None
-            elif request.endpoint and 'diff_history_page' in request.endpoint and datastore.data['settings']['application'].get('shared_diff_access'):
+            elif request.endpoint in SHARED_DIFF_READ_ONLY_ENDPOINTS and datastore.data['settings']['application'].get('shared_diff_access'):
                 return None
             elif request.method in flask_login.config.EXEMPT_METHODS:
                 return None

changedetectionio/tests/test_access_control.py

@@ -48,6 +48,32 @@ def test_check_access_control(app, client, live_server, measure_memory_usage, da
         res = c.get(url_for("ui.ui_diff.diff_history_page", uuid="first"))
         assert b'Random content' in res.data
 
+        # GHSA-vwgh-2hvh-4xm5: shared_diff_access only covers the read-only
+        # diff page — the extract endpoints (which run an attacker-supplied
+        # regex against history and write a CSV to disk) must still require
+        # auth even when the share flag is enabled.
+        res = c.get(url_for("ui.ui_diff.diff_history_page_extract_GET", uuid="first"))
+        assert res.status_code == 302, "Extract form GET must redirect to login for anonymous users"
+        assert b'/login' in res.data or b'login' in res.headers.get('Location', '').encode()
+
+        res = c.post(
+            url_for("ui.ui_diff.diff_history_page_extract_POST", uuid="first"),
+            data={"extract_regex": ".*", "extract_submit_button": "Extract as CSV"},
+        )
+        assert res.status_code == 302, "Extract POST must redirect to login for anonymous users"
+        assert b'login' in res.headers.get('Location', '').encode()
+
+        # But sub-resources the diff page legitimately loads should still pass the gate.
+        # download_patch is linked from diff.html — anonymous viewers must be able to fetch it.
+        # (We don't care about the body here, just that auth doesn't block it.)
+        res = c.get(url_for("ui.ui_diff.download_patch", uuid="first"))
+        assert res.status_code != 302, "download_patch must be reachable for shared diff viewers"
+
+        # processor_asset (used for screenshots embedded in image_ssim_diff watches) must also be reachable.
+        # For a text watch the processor has no such asset so 404 is fine — what matters is no auth redirect.
+        res = c.get(url_for("ui.ui_diff.processor_asset", uuid="first", asset_name="before"))
+        assert res.status_code != 302, "processor_asset must be reachable for shared diff viewers"
+
         # access to assets should work (check_authentication)
         res = c.get(url_for('static_content', group='js', filename='jquery-3.6.0.min.js'))
         assert res.status_code == 200