From 1b47e5abe113eb08ef1d7d3a98fa6728f9e56daf Mon Sep 17 00:00:00 2001 From: Guido Date: Thu, 27 Aug 2026 15:07:42 +0200 Subject: [PATCH] fix: Ohne Anmeldung liessen sich beliebige Dateien vom Server lesen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Die Auslieferung des Frontends nahm den Pfad roh aus der Adresse: "Path / '../../x'" verlaesst den Ordner klaglos, und die Route liegt VOR jeder Anmeldepruefung. Gegen die eigene Instanz nachgestellt – roh und prozentkodiert kam beides Mal die Datei heraus: GET /../../GEHEIM.txt -> "geheim" GET /../../backend/app/config.py -> der Quelltext Damit war alles lesbar, was der Dienst lesen darf: Quelltext, Konfiguration, und mit der passenden Zahl von Ebenen die Datenbank samt verschluesseltem Postfach-Passwort und Bank-Schluessel. Der Pfad wird jetzt aufgeloest und muss im Auslieferungs-Ordner liegen. Zweitens prueft der Postfach-Abruf jetzt das Zertifikat. Ohne eigenen SSL-Kontext tut imaplib das NICHT – die Verschluesselung schuetzt dann vor niemandem, der sich dazwischenhaengt, und das Postfach-Passwort geht mit. Gilt fuer beide Wege, SSL und STARTTLS. Drittens warnt der Start, wenn das Sitzungs-Cookie ohne "Secure" ausgeliefert wird. Die Vorgabe bleibt bewusst aus: Sie umzustellen sperrt jeden aus, der Certain im Heimnetz ueber HTTP aufruft – und ein Ausgesperrter saehe die Ursache nirgends. Die Befunde stammen aus dem /security-review dieser Sitzung und waren seither offen. Sie wiegen schwerer, seit die Anwendung fuer die Bankanbindung unter einer oeffentlichen HTTPS-Adresse erreichbar sein muss. Co-Authored-By: Claude Opus 5 --- backend/app/main.py | 27 ++++++++++-- backend/app/services/mail_import.py | 12 +++-- backend/tests/test_isolation.py | 68 +++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/backend/app/main.py b/backend/app/main.py index 0f05d35..723c060 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -54,6 +54,8 @@ settings = get_settings() # Pfad zum gebauten Frontend (im Dev von Vite selbst ausgeliefert -> fehlt hier). REPO_ROOT = Path(__file__).resolve().parent.parent.parent DIST_DIR = REPO_ROOT / "web" / "dist" +# Einmal aufgeloest: Gegen diesen Pfad wird jede Anfrage geprueft. +DIST_WURZEL = DIST_DIR.resolve() @asynccontextmanager @@ -66,6 +68,17 @@ async def lifespan(app: FastAPI): finally: db.close() + # Das Sitzungs-Cookie ohne „Secure" wandert auch über eine unverschlüsselte + # Verbindung – wer mitliest, ist angemeldet. Die Vorgabe bleibt trotzdem + # aus: Sie umzustellen sperrt jeden aus, der die App im Heimnetz über HTTP + # aufruft, und ein Ausgesperrter sieht die Ursache nirgends. + if not settings.cookie_secure: + log.warning( + "APP_COOKIE_SECURE ist aus – das Sitzungs-Cookie geht auch über " + "unverschlüsselte Verbindungen. Wenn Certain über HTTPS erreichbar " + "ist (und für die Bankanbindung MUSS es das), setz es auf 1." + ) + # EZB-Kurse im HINTERGRUND laden – der Start darf nie darauf warten # (langsames/geblocktes Netz oder hängendes DNS würde den Dienst sonst # blockieren und der Health-Check nie grün werden). @@ -263,7 +276,15 @@ if DIST_DIR.exists(): # /api-Pfade, die keine echte Route trafen, sind echte 404 (kein SPA). if full_path.startswith("api"): raise HTTPException(status_code=404, detail="Not Found") - candidate = DIST_DIR / full_path - if full_path and candidate.is_file(): - return FileResponse(candidate) + # Aufgeloest UND geprueft, ob die Datei noch im Auslieferungs-Ordner + # liegt. Ohne das liefert "/../../etc/passwd" genau das aus: Der Pfad + # kommt roh aus der Adresse, und "Path / '../x'" verlaesst den Ordner + # klaglos. Ohne Anmeldung, denn diese Route liegt vor jeder Pruefung. + try: + kandidat = (DIST_DIR / full_path).resolve() + except (OSError, ValueError): + kandidat = None + if (full_path and kandidat is not None and kandidat.is_file() + and kandidat.is_relative_to(DIST_WURZEL)): + return FileResponse(kandidat) return FileResponse(DIST_DIR / "index.html", headers={"Cache-Control": "no-cache"}) diff --git a/backend/app/services/mail_import.py b/backend/app/services/mail_import.py index 3403fab..f5cb230 100644 --- a/backend/app/services/mail_import.py +++ b/backend/app/services/mail_import.py @@ -17,6 +17,7 @@ import email import email.policy import email.utils import imaplib +import ssl import logging from datetime import date, datetime, timedelta, timezone @@ -49,12 +50,17 @@ def _verbinden(quelle: MailSource) -> imaplib.IMAP4: try: if art == "ssl": # TLS ab der ersten Zeile – der Normalfall auf Port 993. - verbindung = imaplib.IMAP4_SSL(quelle.host, quelle.port) + # MIT eigenem Kontext: Ohne einen prueft imaplib das Zertifikat + # nicht, und die Verschluesselung schuetzt dann vor niemandem, der + # sich dazwischenhaengt – das Postfach-Passwort geht mit. + verbindung = imaplib.IMAP4_SSL(quelle.host, quelle.port, + ssl_context=ssl.create_default_context()) else: verbindung = imaplib.IMAP4(quelle.host, quelle.port) if art == "starttls": - # Erst im Klartext verbinden, dann verschlüsseln (Port 143). - verbindung.starttls() + # Erst im Klartext verbinden, dann verschlüsseln (Port 143) – + # ebenfalls mit geprüftem Zertifikat, aus demselben Grund. + verbindung.starttls(ssl.create_default_context()) except OSError as e: raise MailFehler(f"Keine Verbindung zu {quelle.host}:{quelle.port} – {e}") from e except imaplib.IMAP4.error as e: diff --git a/backend/tests/test_isolation.py b/backend/tests/test_isolation.py index a60c0ac..99a07bd 100644 --- a/backend/tests/test_isolation.py +++ b/backend/tests/test_isolation.py @@ -25,3 +25,71 @@ def test_users_are_isolated(admin): assert len(other.get("/api/categories").json()) >= 6 # Kein Admin: darf keine Nutzer listen. assert other.get("/api/users").status_code == 403 + + +def test_kein_pfadwechsel_aus_dem_auslieferungs_ordner(client, tmp_path): + """Diese Route liegt VOR jeder Anmeldepruefung. Ohne Begrenzung liefert + "/../../etc/passwd" genau das aus – der Pfad kommt roh aus der Adresse, + und "Path / '../x'" verlaesst den Ordner klaglos.""" + from app import main + + if not main.DIST_DIR.exists(): + import pytest + pytest.skip("kein gebautes Frontend vorhanden") + + geheim = main.DIST_DIR.parent.parent / "PRUEF-GEHEIM.txt" + geheim.write_text("streng geheim", encoding="utf-8") + try: + for pfad in ("/../../PRUEF-GEHEIM.txt", + "/%2e%2e%2f%2e%2e%2fPRUEF-GEHEIM.txt", + "/../../backend/app/config.py"): + r = client.get(pfad) + assert "streng geheim" not in r.text, f"{pfad} gab die Datei heraus" + assert "APP_DATA_DIR" not in r.text, f"{pfad} gab Quelltext heraus" + finally: + geheim.unlink(missing_ok=True) + + +def test_echte_dateien_werden_weiterhin_ausgeliefert(client): + """Die Begrenzung darf die Auslieferung nicht kaputtmachen.""" + from app import main + + if not (main.DIST_DIR / "index.html").exists(): + import pytest + pytest.skip("kein gebautes Frontend vorhanden") + assert client.get("/").status_code == 200 + assert "