cobb/cauldron
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

security: fix 2 CRITs surfaced by 2nd-pass audit (CSRF prefix bypass + slug XSS)

Browse source 
Both findings are net-new — first audit ran before the CSRF guard existed,
and the picks→onclick interpolation predates the discover/scrape work that
expanded slug shape. Verified by grep+read.

server.py CVE-NEW-1 (CRIT): @before_request CSRF guard used
`origin.startswith(cfg.base_url)`. With CAULDRON_BASE_URL=https://cauldron.sulkta.com,
an attacker-registered `cauldron.sulkta.com.evil.com` produces an
Origin header that startswith the configured base — guard passes,
A1 fix trivially defeated. Replaced with parsed-origin equality
(scheme+netloc, byte-exact at the netloc boundary). Pre-parse the
expected origin once at app boot rather than per-request.

server.py + picks.html CVE-NEW-2 (CRIT): /api/picks/<slug> took
slug from URL path with NO validation against the household's
recipe index. picks.html then interpolated slug into
`onclick="removePick('{{ slug }}', this)"` — Jinja escapes `'` to
`&#39;` but HTML attribute decoding returns bare `'` to the JS
engine, so a slug like `x'),alert(1);//` round-trips DB → template
→ JS execution for every household member who loads /picks. Two-
layer fix:
  - add_pick now requires slug ∈ db.find_indexed_recipe(hid, slug),
    returning 404 on miss. Also closes prompt-injection-via-poison-
    slug into the planner. Indexed name is trusted over client-
    supplied name (defense in depth on the name field too).
  - picks.html switches to a delegated click listener reading
    slug from the parent <li>'s data-slug attribute. Slug never
    lands inside a JS string literal in HTML.

server.py CVE-NEW-3 (HIGH): _safe_next() helper centralizes the
post-login redirect validation. Applied at BOTH /login stash time
AND /auth/callback consumption time so a future writer of
session['post_login_next'] can't bypass. Strict path charset
[A-Za-z0-9_./-], rejects scheme/netloc, rejects `//`, `/\`.
This commit is contained in:
Kayos 2026-05-02 16:48:21 -07:00
parent 5c60b7a115
commit fdd1102a6f
2 changed files with 106 additions and 39 deletions
Download patch file Download diff file Expand all files Collapse all files

125
cauldron/server.py
View file

@ -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/<arbitrary-string> 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 `<a>` 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})

20
cauldron/templates/picks.html
View file

@ -26,7 +26,7 @@
<div style="display: flex; justify-content: space-between; align-items: baseline; gap: 14px;">
<a href="/recipes/{{ p.slug }}" style="flex: 1; color: var(--bone); font-family: var(--serif); font-size: 1.05em; border: none;">{{ p.name }}</a>
{% if p.mine %}
<button class="btn" type="button" onclick="removePick('{{ p.slug }}', this)" style="font-size: 11px; padding: .35em .9em;">unpin</button>
<button class="btn js-unpin" type="button" style="font-size: 11px; padding: .35em .9em;">unpin</button>
{% endif %}
</div>
<div style="margin-top: 4px; color: var(--muted); font-size: 11px; letter-spacing: .1em; text-transform: uppercase; font-family: var(--mono);">
@ -53,18 +53,28 @@
{% endif %}
<script>
async function removePick(slug, btn) {
// Delegated unpin listener — slug is read from the parent <li>'s
// data-slug attribute (HTML-attribute context, autoescaped by Jinja),
// never interpolated into a JS string literal inside HTML. Audit
// CVE-NEW-2 fix 2026-05-02 PM: the prior `onclick="removePick('{{ slug }}',...)"`
// pattern was a stored-XSS surface because HTML attribute decoding
// returns the bare `'` to the JS engine.
document.getElementById('picks-list')?.addEventListener('click', async (ev) => {
const btn = ev.target.closest('.js-unpin');
if (!btn) return;
const li = btn.closest('li');
const slug = li?.dataset?.slug;
if (!slug) return;
btn.disabled = true; btn.textContent = '…';
try {
const r = await fetch(`/api/picks/${encodeURIComponent(slug)}`, { method: 'DELETE' });
if (!r.ok) throw new Error(r.status);
const li = btn.closest('li');
if (li) li.remove();
li.remove();
if (!document.querySelectorAll('#picks-list li').length) location.reload();
} catch (e) {
btn.disabled = false; btn.textContent = 'unpin';
}
}
});
</script>
{% endblock %}