diff --git a/cauldron/server.py b/cauldron/server.py index f7d5065..eeff745 100644 --- a/cauldron/server.py +++ b/cauldron/server.py @@ -24,6 +24,7 @@ Routes (current): import hmac from datetime import date, datetime, timedelta from functools import wraps +from urllib.parse import urlparse import requests from authlib.integrations.base_client.errors import MismatchingStateError, OAuthError @@ -177,29 +178,50 @@ def create_app() -> Flask: # SAMESITE=Lax alone doesn't cover same-site subdomain CSRF (a # compromised *.sulkta.com page POSTing to cauldron.sulkta.com # carries cookies). When CAULDRON_BASE_URL is set, every state- - # mutating request must have an Origin (or Referer) that starts - # with that base. Bearer-token API calls are exempt — no cookie - # means no CSRF surface. Pure-GET/HEAD/OPTIONS are exempt. + # mutating request must EXACTLY match origin (scheme+host+port). + # Bearer-token API calls are exempt — no cookie means no CSRF + # surface. Pure-GET/HEAD/OPTIONS are exempt. + # + # 2nd-pass audit fix (2026-05-02 PM, CVE-NEW-1): the original guard + # used `startswith(cfg.base_url)` which is bypassable by an attacker + # registering `cauldron.sulkta.com.evil.com` — its Origin string + # starts-with `https://cauldron.sulkta.com`. Switched to parsed- + # origin equality so the host comparison is byte-exact at the + # netloc boundary. + _expected_origin = "" + if cfg.base_url: + _bp = urlparse(cfg.base_url) + if _bp.scheme and _bp.netloc: + _expected_origin = f"{_bp.scheme}://{_bp.netloc}" + + def _origin_of(url: str) -> str: + if not url: + return "" + try: + p = urlparse(url) + except Exception: + return "" + if not p.scheme or not p.netloc: + return "" + return f"{p.scheme}://{p.netloc}" + @app.before_request def _csrf_origin_guard(): - if not cfg.base_url: + if not _expected_origin: return # LAN-only deploy — same-origin is implicit if request.method in ("GET", "HEAD", "OPTIONS"): return # Bearer-auth callers don't carry cookies → no CSRF if request.headers.get("Authorization", "").startswith("Bearer "): return - # Origin is the canonical signal; Referer is a fallback - origin = request.headers.get("Origin") or "" - referer = request.headers.get("Referer") or "" - ok = ( - (origin and origin.startswith(cfg.base_url)) - or (referer and referer.startswith(cfg.base_url)) - ) - if not ok: + origin_hdr = request.headers.get("Origin") or "" + referer_hdr = request.headers.get("Referer") or "" + origin_match = origin_hdr and _origin_of(origin_hdr) == _expected_origin + referer_match = referer_hdr and _origin_of(referer_hdr) == _expected_origin + if not (origin_match or referer_match): app.logger.warning( "csrf reject: method=%s path=%s origin=%r referer=%r", - request.method, request.path, origin[:100], referer[:100], + request.method, request.path, origin_hdr[:100], referer_hdr[:100], ) if request.path.startswith("/api/"): return jsonify({"error": "csrf_origin_mismatch"}), 403 @@ -403,16 +425,42 @@ def create_app() -> Flask: return redirect(url_for("login")) return redirect(url_for("me")) + def _safe_next(nxt: str | None) -> str: + """Validate a post-login redirect target is a same-origin local + path. Defense-in-depth open-redirect guard — we apply this BOTH + at the /login stash AND at /auth/callback consumption (CVE-NEW-3 + audit fix 2026-05-02 PM). The double-check protects against any + future code path that writes session['post_login_next'] outside + of /login, and against percent-encoded path tricks.""" + if not nxt: + return "/me" + # Must start with `/` and only `/`. Reject `//foo`, `/\\foo`, + # any scheme/host. Reject if urlparse extracts a netloc. + if not nxt.startswith("/"): + return "/me" + if nxt.startswith("//") or nxt.startswith("/\\"): + return "/me" + try: + p = urlparse(nxt) + except Exception: + return "/me" + if p.scheme or p.netloc: + return "/me" + # Allow only a strict path charset. Anything weirder lands at /me. + # Path component is everything before the optional `?` / `#`. + path = p.path or "/" + for ch in path: + if not (ch.isalnum() or ch in "-_./"): + return "/me" + return nxt + @app.get("/login") def login(): - # Stash where to go after login. Validate same-origin path to - # close the open-redirect surface — `next=https://evil.example/...` - # would otherwise route an authenticated user to an attacker page - # right after OIDC handshake. Audit F-3a routes 2026-05-02. - nxt = request.args.get("next") or "/me" - if not nxt.startswith("/") or nxt.startswith("//") or nxt.startswith("/\\"): - nxt = "/me" - session["post_login_next"] = nxt + # Stash where to go after login. _safe_next closes the open- + # redirect surface — `next=https://evil.example/...` would + # otherwise route an authenticated user to an attacker page + # right after OIDC handshake. + session["post_login_next"] = _safe_next(request.args.get("next")) return oauth.cauldron.authorize_redirect(cfg.oidc_redirect_uri) @app.get("/auth/callback") @@ -463,7 +511,8 @@ def create_app() -> Flask: # session cookie (no Expires) and tab-close kills it. Audit # CVE-D2 (2026-05-02). session.permanent = True - return redirect(session.pop("post_login_next", "/me")) + # Re-validate post_login_next at consumption (CVE-NEW-3 fix). + return redirect(_safe_next(session.pop("post_login_next", None))) @app.post("/logout") def logout(): @@ -693,18 +742,26 @@ def create_app() -> Flask: @require_session def add_pick(slug: str): u = session["user"] - name = (request.json or {}).get("name", "") if request.is_json else request.form.get("name", "") - if not name: - # Look it up from Mealie if missing - client = current_user_mealie() - if client: - try: - r = client.get_recipe(slug) - name = r.get("name") or slug - except Exception: - name = slug - else: - name = slug + # Validate slug exists in this household's recipe index BEFORE + # accepting the pin. Without this, a crafted POST to + # /api/picks/ stored junk in cauldron_meal_picks + # — and `picks.html` interpolated the slug straight into a JS + # `onclick='removePick('{{ slug }}', ...)'` literal, opening a + # stored-XSS surface where any household member viewing /picks + # ran the attacker's JS. Audit CVE-NEW-2 (2026-05-02 PM 2nd-pass). + # Also closes the prompt-injection-via-poison-slug vector since + # the planner would otherwise pass garbage slugs to Sonnet. + hid = current_household_id() + if hid is None: + return jsonify({"ok": False, "error": "no_household"}), 400 + idx = db.find_indexed_recipe(hid, slug) + if not idx: + return jsonify({"ok": False, "error": "recipe_not_indexed"}), 404 + # Trust the indexed name over client-supplied — also closes the + # XSS surface for the `name` field if the template ever interpolates + # it into JS (currently only renders inside `` text where Jinja + # autoescape is sufficient, but defense in depth). + name = idx.get("name") or slug added = db.add_meal_pick(u["sub"], slug, name) return jsonify({"ok": True, "added": added, "slug": slug}) diff --git a/cauldron/templates/picks.html b/cauldron/templates/picks.html index 414eaed..b3ad6ac 100644 --- a/cauldron/templates/picks.html +++ b/cauldron/templates/picks.html @@ -26,7 +26,7 @@
{{ p.name }} {% if p.mine %} - + {% endif %}
@@ -53,18 +53,28 @@ {% endif %} {% endblock %}