GHSA-23mp-8222-96fr - reflected XSS in /diff/<uuid>/download-patch via from_version - Using built-in plain text response instead. Reported-by: Mayssare Amakhtari (@cy3erm)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (#4324)

changedetectionio/blueprint/__init__.py

@@ -0,0 +1,20 @@
+from flask import make_response
+
+
+def plaintext_response(message, status):
+    """
+    Error response for a body that may contain caller-supplied text.
+
+    Flask's make_response() defaults to Content-Type: text/html, so any user input echoed
+    into an error body becomes reflected XSS. That is the failure behind
+    GHSA-23mp-8222-96fr (/diff/<uuid>/download-patch), CVE-2026-27645 (/rss/watch/) and
+    CVE-2026-29038 (/rss/tag/) - three instances of one pattern. Forcing text/plain means
+    the browser will not parse the body as markup even if input does reach it.
+
+    Prefer validating input over relying on this, but use it on error paths regardless:
+    exception text routinely carries values nobody audited (selectors, timestamps,
+    filesystem paths from snapshot reads).
+    """
+    response = make_response(message, status)
+    response.headers['Content-Type'] = 'text/plain; charset=utf-8'
+    return response

changedetectionio/blueprint/browser_steps/__init__.py

@@ -17,6 +17,7 @@ from flask import Blueprint, request, make_response
 import os
 
 from changedetectionio.store import ChangeDetectionStore
+from changedetectionio.blueprint import plaintext_response
 from changedetectionio.flask_app import login_optionally_required
 from changedetectionio.validate_url import validate_fetch_url_async
 from loguru import logger
@@ -400,8 +401,10 @@ def construct_blueprint(datastore: ChangeDetectionStore):
 
         except Exception as e:
             logger.error(f"Exception when calling step operation {step_operation} {str(e)}")
-            # Try to find something of value to give back to the user
-            return make_response(str(e).splitlines()[0], 401)
+            # Try to find something of value to give back to the user.
+            # text/plain: the message can contain the user's own selectors/values, so it
+            # must not be parsed as HTML by the browser (GHSA-23mp-8222-96fr pattern).
+            return plaintext_response(str(e).splitlines()[0], 401)
 
         # Screenshots and other info only needed on requesting a step (POST)
         try:
@@ -418,7 +421,7 @@ def construct_blueprint(datastore: ChangeDetectionStore):
                     watch.save_xpath_data(data=xpath_data)
 
         except Exception as e:
-            return make_response(f"Error fetching screenshot and element data - {str(e)}", 401)
+            return plaintext_response(f"Error fetching screenshot and element data - {str(e)}", 401)
 
         # SEND THIS BACK TO THE BROWSER
         output = {

changedetectionio/blueprint/ui/diff.py

@@ -14,6 +14,7 @@ from changedetectionio.diff import (
     CHANGED_INTO_PLACEMARKER_OPEN, CHANGED_INTO_PLACEMARKER_CLOSED
 )
 from changedetectionio.store import ChangeDetectionStore
+from changedetectionio.blueprint import plaintext_response
 from changedetectionio.auth_decorator import login_optionally_required
 
 
@@ -516,20 +517,36 @@ def construct_blueprint(datastore: ChangeDetectionStore):
         try:
             watch = datastore.data['watching'][uuid]
         except KeyError:
-            return make_response('Watch not found', 404)
+            return plaintext_response('Watch not found', 404)
 
         dates = list(watch.history.keys())
         if len(dates) < 2:
-            return make_response('Not enough history', 400)
+            return plaintext_response('Not enough history', 400)
 
         from_version = request.args.get('from_version', dates[-2])
         to_version   = request.args.get('to_version',   dates[-1])
 
+        # Validate before use. get_history_snapshot() does a plain dict lookup, so an
+        # unknown key raises KeyError whose str() carries the raw query parameter - which
+        # the error path below used to reflect into a text/html response, giving reflected
+        # XSS (GHSA-23mp-8222-96fr). This mirrors the check the API resource already
+        # performs for the same operation in api/Watch.py.
+        for timestamp in (from_version, to_version):
+            if timestamp not in watch.history:
+                return plaintext_response('Snapshot not found', 404)
+
         try:
             from_text = watch.get_history_snapshot(timestamp=from_version)
             to_text   = watch.get_history_snapshot(timestamp=to_version)
         except Exception as e:
-            return make_response(f'Could not read snapshots: {e}', 500)
+            # Never reflect the exception text. Besides the KeyError guarded above, this
+            # also catches brotli decode failures and the data-dir containment guard in
+            # get_history_snapshot(), whose messages carry filesystem paths. Send the
+            # detail to the log and keep the response text/plain regardless, matching the
+            # pattern already applied to rss/single_watch.py, so nothing that reaches the
+            # body can be parsed as HTML by the browser.
+            logger.error(f"download-patch: could not read snapshots for watch {uuid}: {e}")
+            return plaintext_response('Could not read snapshots', 500)
 
         diff_lines = list(difflib.unified_diff(
             from_text.splitlines(keepends=True),

changedetectionio/tests/test_diff_download_patch_xss.py

@@ -0,0 +1,86 @@
+#!/usr/bin/env python3
+"""
+Reflected XSS regression tests for /diff/<uuid>/download-patch (GHSA-23mp-8222-96fr).
+
+get_history_snapshot() does a plain dict lookup on the history, so an unknown timestamp
+raised KeyError whose str() carries the raw from_version/to_version query parameter. The
+error path interpolated that into make_response(), which defaults to text/html - so the
+parameter was reflected unescaped and executed in the victim's browser.
+
+Two independent guarantees are asserted here, because either alone would have prevented
+this bug and both are worth keeping:
+  1. Unknown timestamps are rejected up front (404), so the exception never fires.
+  2. Whatever the error path returns is text/plain and never echoes the input.
+
+This is the third instance of the same pattern, after CVE-2026-27645 (/rss/watch/) and
+CVE-2026-29038 (/rss/tag/).
+"""
+
+import os
+from flask import url_for
+from .util import wait_for_all_checks
+
+XSS_PAYLOADS = [
+    '<svg/onload=alert(document.domain)>',
+    '"><script>alert(1)</script>',
+    "'><img src=x onerror=alert(1)>",
+]
+
+
+def _setup_watch_with_history(client, datastore_path):
+    """A watch needs >=2 snapshots before download-patch will get as far as the lookup."""
+    with open(os.path.join(datastore_path, "endpoint-content.txt"), "w") as f:
+        f.write("first content")
+
+    test_url = url_for('test_endpoint', _external=True)
+    uuid = client.application.config.get('DATASTORE').add_watch(url=test_url)
+    client.post(url_for("ui.form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    with open(os.path.join(datastore_path, "endpoint-content.txt"), "w") as f:
+        f.write("second content, now different")
+    client.post(url_for("ui.form_watch_checknow"), follow_redirects=True)
+    wait_for_all_checks(client)
+
+    return uuid
+
+
+def test_download_patch_does_not_reflect_unknown_timestamp(client, live_server, measure_memory_usage, datastore_path):
+    uuid = _setup_watch_with_history(client, datastore_path)
+
+    for payload in XSS_PAYLOADS:
+        # from_version and to_version are separate code paths; both must be validated
+        for param in ('from_version', 'to_version'):
+            res = client.get(url_for("ui.ui_diff.download_patch", uuid=uuid, **{param: payload}))
+
+            assert payload.encode() not in res.data, \
+                f"{param}={payload!r} was reflected into the response body"
+
+            # An unknown timestamp is a client error, not a 500 from an unhandled KeyError
+            assert res.status_code == 404, \
+                f"{param}={payload!r} should be rejected up front, got {res.status_code}"
+
+            # Even if a future refactor lets the exception path run again, the body must
+            # not be parseable as HTML
+            if res.status_code >= 400:
+                assert 'text/html' not in res.headers.get('Content-Type', ''), \
+                    f"error response for {param}={payload!r} should not be text/html"
+
+
+def test_download_patch_still_works_for_valid_timestamps(client, live_server, measure_memory_usage, datastore_path):
+    """The validation must not break the legitimate path."""
+    uuid = _setup_watch_with_history(client, datastore_path)
+    watch = client.application.config.get('DATASTORE').data['watching'][uuid]
+    dates = list(watch.history.keys())
+    assert len(dates) >= 2
+
+    res = client.get(url_for("ui.ui_diff.download_patch", uuid=uuid,
+                             from_version=dates[-2], to_version=dates[-1]))
+    assert res.status_code == 200
+    assert 'text/plain' in res.headers.get('Content-Type', '')
+    assert b'second content' in res.data, "Patch should contain the changed text"
+
+    # And with no params at all it should default to the last two snapshots
+    res = client.get(url_for("ui.ui_diff.download_patch", uuid=uuid))
+    assert res.status_code == 200
+    assert b'second content' in res.data