Reject protocol-relative URLs in next redirect validation

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(_safe_next(url_for("dashboard")))
-        return redirect(next_url if next_url.startswith("/") else 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", "")
+        return redirect(_safe_next(url_for("device_list")))
-        if nxt.startswith("/"):
-            return redirect(nxt)
-        return redirect(url_for("device_list"))
 
     # ------------------------------------------------------------------ #
     # Devices — batch install specific batteries

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
 # ------------------------------------------------------------------ #