fix(security): batch-2 hardening — XSS allowlist, event-scoping, deploy

Address the seven findings from the security & deployment review.

High:
- upload: make infer authoritative — reject files it can't identify
  (SVG/HTML/JS) and require the detected MIME to be on an ALLOWED_MEDIA
  allowlist; derive the stored MIME and on-disk extension from the
  detected type, ignoring client filename/Content-Type. Closes the
  stored-XSS vector via media served on-origin.
- deploy: rename docker-compose.override.yml -> docker-compose.dev.yml
  so the default `docker compose up -d` no longer publishes Postgres
  5432 to the host; the port map is now opt-in via -f. README updated.

Medium:
- upload: DefaultBodyLimit::disable() -> max(576 MiB) as an HTTP-level
  OOM backstop; handler still enforces precise per-class size limits.
- docker: run backend and frontend as non-root users.

Low:
- social/upload: event-scope toggle_like, list_comments, add_comment,
  delete_comment, edit_upload, delete_upload via find_by_id_and_event /
  soft_delete_in_event — cross-event IDs now resolve to 404.
- Caddy: site-wide HSTS / nosniff / X-Frame-Options / Referrer-Policy,
  plus Content-Disposition: attachment on /media/originals/*.
- .env.example: replace default Postgres password with a CHANGE_ME hint.

Out of scope: localStorage JWT (root cause fixed; httpOnly cookies are a
larger change tracked separately).

Verified: cargo build (no new warnings), cargo test (3 passed),
caddy validate, docker compose config (no 5432 published by default).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
fabi
2026-06-29 19:39:29 +02:00
parent bbec815854
commit edcef0258c
10 changed files with 134 additions and 58 deletions

View File

@@ -20,8 +20,16 @@ FROM alpine:3.21
RUN apk add --no-cache ca-certificates ffmpeg
# Run as a non-root user. Pre-create and chown the media mount path so the fresh
# named volume inherits the non-root ownership (Docker seeds an empty named volume
# from the image directory, preserving its uid/gid) and uploads can be written.
RUN addgroup -S app && adduser -S app -G app
WORKDIR /app
COPY --from=builder /app/target/release/eventsnap-backend ./
RUN mkdir -p /media && chown -R app:app /app /media
USER app
EXPOSE 3000
CMD ["./eventsnap-backend"]

View File

@@ -9,6 +9,7 @@ use crate::auth::middleware::AuthUser;
use crate::error::AppError;
use crate::models::comment::{Comment, CommentDto};
use crate::models::hashtag::{self, Hashtag};
use crate::models::upload::Upload;
use crate::state::AppState;
pub async fn toggle_like(
@@ -24,6 +25,12 @@ pub async fn toggle_like(
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
}
// Event-scope: the upload must belong to the caller's event (404 otherwise),
// matching the host handlers' find_by_id_and_event pattern.
Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
.await?
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
// Try to insert; if conflict, delete (toggle)
let result = sqlx::query(
"INSERT INTO \"like\" (upload_id, user_id) VALUES ($1, $2)
@@ -64,10 +71,15 @@ const COMMENT_PAGE_SIZE: i64 = 50;
pub async fn list_comments(
State(state): State<AppState>,
_auth: AuthUser,
auth: AuthUser,
Path(upload_id): Path<Uuid>,
Query(q): Query<ListCommentsQuery>,
) -> Result<Json<Vec<CommentDto>>, AppError> {
// Event-scope: only list comments for an upload in the caller's event.
Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
.await?
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
let comments =
Comment::list_for_upload(&state.pool, upload_id, q.before, COMMENT_PAGE_SIZE).await?;
Ok(Json(comments))
@@ -91,6 +103,11 @@ pub async fn add_comment(
return Err(AppError::Forbidden("Du bist gesperrt.".into()));
}
// Event-scope: only comment on an upload that belongs to the caller's event.
Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
.await?
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
let text = body.body.trim();
let text_chars = text.chars().count();
if text_chars == 0 || text_chars > 500 {
@@ -145,6 +162,11 @@ pub async fn delete_comment(
return Err(AppError::Forbidden("Nur eigene Kommentare löschen.".into()));
}
Comment::soft_delete(&state.pool, comment_id).await?;
// Event-scope: soft_delete_in_event only matches comments whose upload is in
// the caller's event, so a cross-event comment_id resolves to a 404 here.
let deleted = Comment::soft_delete_in_event(&state.pool, comment_id, auth.event_id).await?;
if !deleted {
return Err(AppError::NotFound("Kommentar nicht gefunden.".into()));
}
Ok(StatusCode::NO_CONTENT)
}

View File

@@ -16,6 +16,23 @@ use crate::state::AppState;
const MAX_CAPTION_LENGTH: usize = 2000;
/// Allowlist of accepted media types, keyed by the MIME that `infer` derives from
/// the file's magic bytes. The detected MIME (not the client-declared one) is what
/// we trust, store, and hand to the compression pipeline — so a text-based payload
/// (SVG/HTML/JS) can never be stored or served on-origin. Each entry maps to the
/// server-controlled file extension we persist the original under.
const ALLOWED_MEDIA: &[(&str, &str)] = &[
("image/jpeg", "jpg"),
("image/png", "png"),
("image/webp", "webp"),
("image/gif", "gif"),
("image/heic", "heic"),
("image/heif", "heif"),
("video/mp4", "mp4"),
("video/quicktime", "mov"),
("video/webm", "webm"),
];
pub async fn upload(
State(state): State<AppState>,
auth: AuthUser,
@@ -62,8 +79,6 @@ pub async fn upload(
let max_video_mb: i64 = config::get_i64(&state.pool, "max_video_size_mb", 500).await;
let mut file_data: Option<Vec<u8>> = None;
let mut file_name: Option<String> = None;
let mut content_type: Option<String> = None;
let mut caption: Option<String> = None;
let mut hashtags_csv: Option<String> = None;
@@ -71,8 +86,9 @@ pub async fn upload(
let name = field.name().unwrap_or_default().to_string();
match name.as_str() {
"file" => {
file_name = field.file_name().map(|s| s.to_string());
content_type = field.content_type().map(|s| s.to_string());
// Note: the client-declared filename and Content-Type are intentionally
// ignored — the stored MIME and extension are derived from the file's
// magic bytes below, so a mislabelled payload can't influence them.
file_data = Some(
field.bytes().await
.map_err(|e| AppError::BadRequest(format!("Datei konnte nicht gelesen werden: {e}")))?
@@ -96,7 +112,6 @@ pub async fn upload(
}
let data = file_data.ok_or_else(|| AppError::BadRequest("Keine Datei hochgeladen.".into()))?;
let mime = content_type.unwrap_or_else(|| "application/octet-stream".to_string());
let size = data.len() as i64;
// Validate caption length. Counted in chars (code points) to match the
@@ -111,22 +126,23 @@ pub async fn upload(
}
}
// Validate file MIME type using magic bytes
let detected_mime = infer::get(&data);
if let Some(detected) = detected_mime {
let detected_type = detected.mime_type();
// Ensure detected type is compatible with declared MIME type
let declared_category = mime.split('/').next().unwrap_or("");
let detected_category = detected_type.split('/').next().unwrap_or("");
// Only reject if categories don't match (e.g., image vs video)
if declared_category != "application" && declared_category != detected_category {
return Err(AppError::BadRequest(format!(
"Dateiinhalt entspricht nicht dem deklarierten Typ. Erwartet: {}, erkannt: {}",
mime, detected_type
)));
}
}
// Determine the file type from its magic bytes and require it to be on the
// allowlist. `infer` returns None for text-based payloads (SVG/HTML/JS), so
// those are rejected outright — closing the stored-XSS vector. Both the MIME
// we persist and the on-disk extension come from the detected type, never from
// client-supplied values.
let kind = infer::get(&data)
.ok_or_else(|| AppError::BadRequest("Dateityp nicht erkannt oder nicht unterstützt.".into()))?;
let (mime, ext) = ALLOWED_MEDIA
.iter()
.find(|(allowed, _)| *allowed == kind.mime_type())
.map(|(m, e)| ((*m).to_string(), *e))
.ok_or_else(|| {
AppError::BadRequest(format!(
"Dateityp wird nicht unterstützt: {}.",
kind.mime_type()
))
})?;
// Validate file size
let max_bytes = if mime.starts_with("video/") {
@@ -159,12 +175,6 @@ pub async fn upload(
}
}
// Determine file extension
let ext = file_name
.as_deref()
.and_then(|n| n.rsplit('.').next())
.unwrap_or(if mime.starts_with("video/") { "mp4" } else { "jpg" });
let upload_id = Uuid::new_v4();
let event_slug = &state.config.event_slug;
let relative_path = format!("originals/{event_slug}/{upload_id}.{ext}");
@@ -257,7 +267,7 @@ pub async fn edit_upload(
Path(upload_id): Path<Uuid>,
Json(body): Json<EditUploadRequest>,
) -> Result<StatusCode, AppError> {
let upload = Upload::find_by_id(&state.pool, upload_id)
let upload = Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
.await?
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
@@ -285,7 +295,7 @@ pub async fn delete_upload(
auth: AuthUser,
Path(upload_id): Path<Uuid>,
) -> Result<StatusCode, AppError> {
let upload = Upload::find_by_id(&state.pool, upload_id)
let upload = Upload::find_by_id_and_event(&state.pool, upload_id, auth.event_id)
.await?
.ok_or_else(|| AppError::NotFound("Upload nicht gefunden.".into()))?;
@@ -293,7 +303,7 @@ pub async fn delete_upload(
return Err(AppError::Forbidden("Nur eigene Uploads löschen.".into()));
}
Upload::soft_delete(&state.pool, upload_id).await?;
Upload::soft_delete_in_event(&state.pool, upload_id, auth.event_id).await?;
Ok(StatusCode::NO_CONTENT)
}

View File

@@ -18,6 +18,10 @@ mod state;
use config::AppConfig;
use state::AppState;
/// Hard HTTP body cap for the upload endpoint (576 MiB). Backstop against
/// memory-exhaustion; precise per-class size limits are enforced in the handler.
const MAX_UPLOAD_BYTES: usize = 576 * 1024 * 1024;
#[tokio::main]
async fn main() -> Result<()> {
dotenvy::dotenv().ok();
@@ -57,9 +61,13 @@ async fn main() -> Result<()> {
.route("/api/v1/recover", post(auth::handlers::recover))
.route("/api/v1/admin/login", post(auth::handlers::admin_login))
.route("/api/v1/session", delete(auth::handlers::logout))
// Upload — body limit disabled; size validation is done inside the handler
// Upload — HTTP-level body cap as an OOM backstop. The handler still enforces
// the precise per-class limits from DB config (max_image/video_size_mb); this
// layer just stops a multi-GB body from being buffered into memory before that
// check runs. Sized generously above the default 500 MB video limit + multipart
// overhead — if an admin raises max_video_size_mb above this, bump MAX_UPLOAD_BYTES.
.route("/api/v1/upload", post(handlers::upload::upload)
.route_layer(DefaultBodyLimit::disable()))
.route_layer(DefaultBodyLimit::max(MAX_UPLOAD_BYTES)))
.route(
"/api/v1/upload/{id}",
patch(handlers::upload::edit_upload).delete(handlers::upload::delete_upload),