diff --git a/changedetectionio/auth_decorator.py b/changedetectionio/auth_decorator.py index 3f495762..096585b7 100644 --- a/changedetectionio/auth_decorator.py +++ b/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//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) diff --git a/changedetectionio/flask_app.py b/changedetectionio/flask_app.py index 0cf45d9c..987309b8 100644 --- a/changedetectionio/flask_app.py +++ b/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 diff --git a/changedetectionio/tests/test_access_control.py b/changedetectionio/tests/test_access_control.py index fc370195..aad85346 100644 --- a/changedetectionio/tests/test_access_control.py +++ b/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