From 7dcf0852303a09bc2941b419fc39e7878adfee4f Mon Sep 17 00:00:00 2001 From: Niclas Overby Date: Tue, 14 Jul 2026 12:29:38 +0200 Subject: [PATCH] fix(web/wiki-dioxus): Harden the backend against SSRF and vote abuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Web push (SSRF): reject any endpoint that is not https or whose host is an IP literal / localhost, at both subscribe and send time. Blocks a registered endpoint pointing at the cloud metadata service or an internal address. The guard rejects no legitimate browser push endpoint; unit-tested. - /vote/cast: the admin-secret path bypasses Hasura row-level security, so the ballot is now authorised here — the target must be an open vote/poll and the caller an active member of that poll's context (context taken from the poll, not the client). Fails closed. Authorisation errors return 403 with a safe message; unrelated internal errors are genericised. - Stop leaking raw Hasura error text to clients (admin_gql logs the detail and returns a generic message) and stop logging the secret path of push endpoints. Signed-off-by: Niclas Overby --- backend/src/auth.rs | 6 +++- backend/src/notify.rs | 14 +++++++- backend/src/push.rs | 61 +++++++++++++++++++++++++++++++++ backend/src/vote.rs | 80 +++++++++++++++++++++++++++++++++++++++---- 4 files changed, 153 insertions(+), 8 deletions(-) diff --git a/backend/src/auth.rs b/backend/src/auth.rs index 9251a591..a0c7e07c 100644 --- a/backend/src/auth.rs +++ b/backend/src/auth.rs @@ -35,7 +35,11 @@ pub async fn admin_gql( .map_err(|e| e.to_string())?; let v: Value = resp.json().await.map_err(|e| e.to_string())?; if let Some(errors) = v.get("errors") { - return Err(format!("hasura error: {errors}")); + // Log the Hasura detail server-side, but return a generic error so query + // internals (schema, constraint names) never reach the client via + // `error_json`. Handler-level domain errors are unaffected. + eprintln!("hasura error: {errors}"); + return Err("backend query failed".into()); } Ok(v) } diff --git a/backend/src/notify.rs b/backend/src/notify.rs index ef305b89..2153a42b 100644 --- a/backend/src/notify.rs +++ b/backend/src/notify.rs @@ -44,6 +44,10 @@ async fn subscribe_inner( let endpoint = get("endpoint") .filter(|s| !s.is_empty()) .ok_or("missing endpoint")?; + // SSRF guard: never store an endpoint the backend must not POST to. + if !push::endpoint_allowed(&endpoint) { + return Err("disallowed push endpoint".into()); + } let p256dh = get("p256dh") .filter(|s| !s.is_empty()) .ok_or("missing p256dh")?; @@ -146,7 +150,15 @@ async fn push_to_emails( match push::send(cfg, client, sub, payload.as_bytes()).await { Ok(status) if (200..300).contains(&status) => sent += 1, Ok(404) | Ok(410) => stale.push(sub.endpoint.clone()), - Ok(status) => eprintln!("push send to {} -> {status}", sub.endpoint), + // Log only the endpoint origin: its path segment is a per-user secret. + Ok(status) => eprintln!( + "push send -> {status} ({})", + sub.endpoint + .split('/') + .take(3) + .collect::>() + .join("/") + ), Err(e) => eprintln!("push send error: {e}"), } } diff --git a/backend/src/push.rs b/backend/src/push.rs index 6dbcca66..5fb69eea 100644 --- a/backend/src/push.rs +++ b/backend/src/push.rs @@ -96,6 +96,38 @@ fn encrypt(p256dh: &str, auth: &str, plaintext: &[u8]) -> Result, String encrypt_with(&as_secret, &salt, &ua_public, &auth, plaintext) } +/// Whether a client-registered push endpoint is safe for the backend to POST to +/// (SSRF guard). Web-push endpoints are always `https://` on public push +/// infrastructure with a DNS hostname (`fcm.googleapis.com`, +/// `*.push.services.mozilla.com`, `*.notify.windows.com`, `web.push.apple.com`), +/// so we reject anything that is not https, any IP-literal host (which blocks +/// the cloud metadata service, loopback, and private ranges), and localhost. +/// This rejects no legitimate browser subscription. Residual (accepted): a +/// hostname that DNS-resolves to an internal address (rebinding) still passes. +pub fn endpoint_allowed(endpoint: &str) -> bool { + let Some(rest) = endpoint.strip_prefix("https://") else { + return false; + }; + // Host is up to the first '/', '?' or '#'; bracketed IPv6 ends at ']'. + let host = if let Some(after) = rest.strip_prefix('[') { + after.split(']').next().unwrap_or("") + } else { + rest.split(['/', ':', '?', '#']).next().unwrap_or("") + }; + if host.is_empty() { + return false; + } + let lower = host.to_ascii_lowercase(); + if lower == "localhost" || lower.ends_with(".localhost") || lower.ends_with(".local") { + return false; + } + // Reject IP-literal hosts outright (real push endpoints use DNS names). + if host.parse::().is_ok() { + return false; + } + true +} + /// The `scheme://host[:port]` origin of a push endpoint, for the VAPID `aud` claim. fn origin_of(endpoint: &str) -> Result { let rest = endpoint @@ -145,6 +177,11 @@ pub async fn send( if cfg.vapid_private.is_empty() { return Err("push not configured".into()); } + // SSRF guard at send time too, in case a disallowed endpoint predates the + // subscribe-time check (defense in depth). + if !endpoint_allowed(&sub.endpoint) { + return Err("disallowed push endpoint".into()); + } let body = encrypt(&sub.p256dh, &sub.auth, payload)?; let auth = vapid_header(cfg, &sub.endpoint, util::now_secs())?; let resp = client @@ -179,6 +216,30 @@ mod tests { const SALT: &str = "DGv6ra1nlYgDCS1FRnbzlw"; const EXPECTED: &str = "DGv6ra1nlYgDCS1FRnbzlwAAEABBBP4z9KsN6nGRTbVYI_c7VJSPQTBtkgcy27mlmlMoZIIgDll6e3vCYLocInmYWAmS6TlzAC8wEqKK6PBru3jl7A_yl95bQpu6cVPTpK4Mqgkf1CXztLVBSt2Ks3oZwbuwXPXLWyouBWLVWGNWQexSgSxsj_Qulcy4a-fN"; + #[test] + fn endpoint_allowlist_blocks_ssrf_targets() { + // Real browser push endpoints (https, DNS host) are allowed. + assert!(endpoint_allowed( + "https://fcm.googleapis.com/fcm/send/abc123" + )); + assert!(endpoint_allowed( + "https://updates.push.services.mozilla.com/wpush/v2/xyz" + )); + assert!(endpoint_allowed("https://web.push.apple.com/QA/def")); + + // SSRF targets and non-https are rejected. + assert!(!endpoint_allowed("http://fcm.googleapis.com/fcm/send/abc")); // not https + assert!(!endpoint_allowed( + "https://169.254.169.254/latest/meta-data/" + )); // metadata + assert!(!endpoint_allowed("https://127.0.0.1/internal")); + assert!(!endpoint_allowed("https://10.0.0.5/x")); + assert!(!endpoint_allowed("https://192.168.1.1/x")); + assert!(!endpoint_allowed("https://localhost/x")); + assert!(!endpoint_allowed("https://[::1]/x")); // IPv6 loopback + assert!(!endpoint_allowed("ftp://fcm.googleapis.com/x")); + } + #[test] fn rfc8291_example_matches() { let as_secret = SecretKey::from_slice(&util::b64url_decode(AS_PRIVATE).unwrap()).unwrap(); diff --git a/backend/src/vote.rs b/backend/src/vote.rs index 31ab51ef..d13d9f58 100644 --- a/backend/src/vote.rs +++ b/backend/src/vote.rs @@ -6,8 +6,13 @@ //! so the ballot is untraceable, while a separate `has_voted(poll_id, user_id)` //! marker enforces one vote per member without linking the marker to the ballot. //! -//! POST /vote/cast?poll=&context=&choices=0,2 (Authorization: Bearer ) -//! GET /vote/status?poll= -> {"voted": bool} +//! The cast is authorised server-side (the admin path bypasses row-level +//! security): the target must be an open `vote/poll` and the caller an active +//! member of that poll's context. The `context` derives from the poll, not the +//! request. +//! +//! POST /vote/cast?poll=&choices=0,2 (Authorization: Bearer ) +//! GET /vote/status?poll= -> {"voted": bool} use crate::oauth::Config; use axum::{body::Body, response::Response}; @@ -26,6 +31,24 @@ pub async fn cast( StatusCode::CONFLICT, json!({ "ok": false, "error": e }).to_string(), ), + // Authorization/state failures carry a safe, user-facing message and a + // 403, so they reach the voter instead of being genericised by + // `error_json` (which is reserved for internal errors). + Err(e) + if matches!( + e.as_str(), + "not a poll" + | "poll closed" + | "poll not found" + | "poll has no context" + | "not a member of this context" + ) => + { + crate::json( + StatusCode::FORBIDDEN, + json!({ "ok": false, "error": e }).to_string(), + ) + } Err(e) => crate::error_json("vote cast", e), } } @@ -43,15 +66,60 @@ async fn cast_inner( .find(|(pk, _)| pk == k) .map(|(_, v)| v.clone()) }; - let token = crate::auth::token_from(query, bearer).ok_or("missing token")?; let poll = get("poll").ok_or("missing poll")?; - let context = get("context").filter(|c| !c.is_empty()); let choices: Vec = get("choices") .unwrap_or_default() .split(',') .filter_map(|s| s.trim().parse().ok()) .collect(); - let uid = crate::nhost::verify_access_token(&token, &cfg.nhost_jwt_secret)?; + // Identify the caller (verifies the JWT and fetches their invite email). + let (uid, email) = crate::auth::caller(cfg, client, query, bearer).await?; + + // Authorize the ballot. The admin-secret path below bypasses Hasura's + // row-level security, so membership + poll state MUST be checked here: the + // target must be an OPEN `vote/poll`, and the caller an active member of that + // poll's context. The context is read from the poll itself (authoritative), + // never trusted from the client. + let poll_v = crate::auth::admin_gql( + cfg, + client, + json!({ + "query": "query($p: uuid!) { node(id: $p) { mimeId mutable contextId } }", + "variables": { "p": poll }, + }), + ) + .await?; + let pnode = poll_v + .pointer("/data/node") + .filter(|n| !n.is_null()) + .ok_or("poll not found")?; + if pnode.get("mimeId").and_then(|m| m.as_str()) != Some("vote/poll") { + return Err("not a poll".into()); + } + if pnode.get("mutable").and_then(|m| m.as_bool()) != Some(true) { + return Err("poll closed".into()); + } + let poll_context = pnode + .get("contextId") + .and_then(|c| c.as_str()) + .filter(|c| !c.is_empty()) + .ok_or("poll has no context")? + .to_string(); + + // Active membership in the poll's context, matched by the durable node_id + // binding or (fallback) the invite email. + let mem_v = crate::auth::admin_gql( + cfg, + client, + json!({ + "query": "query($c: uuid!, $u: uuid!, $e: String!) { members(where: {parentId: {_eq: $c}, active: {_eq: true}, _or: [{nodeId: {_eq: $u}}, {email: {_eq: $e}}]}, limit: 1) { id } }", + "variables": { "c": poll_context, "u": uid, "e": email }, + }), + ) + .await?; + if mem_v.pointer("/data/members/0").is_none() { + return Err("not a member of this context".into()); + } // Dedup first: insert the has_voted marker. A conflict (0 rows) = already voted. let marker = json!({ @@ -75,7 +143,7 @@ async fn cast_inner( "key": format!("vote-{secs}-{}", crate::util::random_token(6)), "mimeId": "vote/vote", "parentId": poll, - "contextId": context, + "contextId": poll_context, "data": choices, }); let insert = json!({ -- 2.51.2