From d1fc80164fc6cf6c70365544e8771d5a2b648966 Mon Sep 17 00:00:00 2001
From: Darek <idarekj@gmail.com>
Date: Wed, 10 Jun 2026 11:45:45 -0500
Subject: [PATCH] Reject protocol-relative URLs in next redirect validation

---
 app.py                   | 17 +++++++++++------
 tests/test_acceptance.py | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/app.py b/app.py
index 81082a3..fe42141 100644
--- a/app.py
+++ b/app.py
@@ -23,6 +23,15 @@ def _parse_date(val: str) -> str | None:
         return None
 
 
+def _safe_next(default_url):
+    """Return the form's `next` URL only if it is a local path (rejects
+    protocol-relative `//host` and `/\\host` redirects)."""
+    nxt = request.form.get("next", "")
+    if nxt.startswith("/") and not nxt.startswith("//") and not nxt.startswith("/\\"):
+        return nxt
+    return default_url
+
+
 def _record_charge(db, battery, date_val, increment, notes):
     """Apply one charge event to battery. Caller must call db.commit()."""
     if increment:
@@ -452,8 +461,7 @@ def create_app(config_object="config"):
         battery.device_id = None
         db.commit()
         flash(f"{battery.label} unassigned and marked available.", "success")
-        next_url = request.form.get("next", "")
-        return redirect(next_url if next_url.startswith("/") else url_for("dashboard"))
+        return redirect(_safe_next(url_for("dashboard")))
 
     # ------------------------------------------------------------------ #
     # Battery — retire
@@ -1082,10 +1090,7 @@ def create_app(config_object="config"):
             f"Unassigned {count} batter{'y' if count == 1 else 'ies'} from {device.name}.",
             "success",
         )
-        nxt = request.form.get("next", "")
-        if nxt.startswith("/"):
-            return redirect(nxt)
-        return redirect(url_for("device_list"))
+        return redirect(_safe_next(url_for("device_list")))
 
     # ------------------------------------------------------------------ #
     # Devices — batch install specific batteries
diff --git a/tests/test_acceptance.py b/tests/test_acceptance.py
index 2455cc0..f8ba54a 100644
--- a/tests/test_acceptance.py
+++ b/tests/test_acceptance.py
@@ -168,6 +168,24 @@ def test_unassign_battery(seeded_client):
     assert b"available" in resp2.data.lower()
 
 
+def test_unassign_next_honors_local_path(seeded_client):
+    seeded_client.post("/battery/1/assign", data={"device_id": "1"})
+    resp = seeded_client.post("/battery/1/unassign", data={"next": "/device/1"})
+    assert get_location(resp) == "/device/1"
+
+
+def test_unassign_next_rejects_external_redirect(seeded_client):
+    seeded_client.post("/battery/1/assign", data={"device_id": "1"})
+    resp = seeded_client.post("/battery/1/unassign", data={"next": "//evil.com/phish"})
+    assert resp.headers["Location"] == "/"
+
+
+def test_unassign_all_next_rejects_external_redirect(seeded_client):
+    seeded_client.post("/battery/1/assign", data={"device_id": "1"})
+    resp = seeded_client.post("/device/1/unassign-all", data={"next": "//evil.com"})
+    assert resp.headers["Location"] == "/device/"
+
+
 # ------------------------------------------------------------------ #
 # Battery — retire
 # ------------------------------------------------------------------ #