fix(upload): server-side MIME/ext allowlist + atomic transactional write
C2: stored-XSS via the application/* MIME bypass is closed — the declared Content-Type and client filename are no longer trusted. New classify() derives both the canonical MIME and a safe extension purely from magic bytes against a strict allowlist (jpeg/png/webp/heic/mp4/mov/webm); anything infer can't positively identify (incl. HTML/SVG script payloads) is rejected. Unit tests cover accept + reject paths. M12: the stored extension can no longer carry '/' or arbitrary client text — it's one of the allowlist's safe constants, ending directory-pollution and the export DoS. M5/M6: quota is now reserved with a single conditional UPDATE that row-locks (no TOCTOU overshoot) and the reservation + row INSERT run in one transaction; the file is written before commit and unlinked on any rollback, so a crash mid-write can no longer leak quota bytes or orphan a row. Upload::create is now executor-generic to run inside the tx. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,35 @@ use crate::state::AppState;
|
|||||||
|
|
||||||
const MAX_CAPTION_LENGTH: usize = 2000;
|
const MAX_CAPTION_LENGTH: usize = 2000;
|
||||||
|
|
||||||
|
/// Derive the canonical MIME type and a *safe* file extension purely from the
|
||||||
|
/// file's magic bytes, against a strict allowlist. The client-declared
|
||||||
|
/// `Content-Type` and the client filename are never trusted: the old code
|
||||||
|
/// skipped validation entirely for `application/*` and copied the extension
|
||||||
|
/// verbatim from the filename, which allowed storing `{uuid}.html`/`.svg` with
|
||||||
|
/// a script payload (stored XSS) and `/`-bearing extensions (path pollution +
|
||||||
|
/// export DoS).
|
||||||
|
///
|
||||||
|
/// `infer` cannot fingerprint HTML/SVG, so the reject-by-default arm is exactly
|
||||||
|
/// what closes the XSS vector — anything not positively identified as an allowed
|
||||||
|
/// raster image or video is refused. Returns `(canonical_mime, safe_ext, is_video)`.
|
||||||
|
fn classify(data: &[u8]) -> Result<(&'static str, &'static str, bool), AppError> {
|
||||||
|
let kind = infer::get(data)
|
||||||
|
.ok_or_else(|| AppError::BadRequest("Dateityp konnte nicht erkannt werden.".into()))?;
|
||||||
|
match kind.mime_type() {
|
||||||
|
"image/jpeg" => Ok(("image/jpeg", "jpg", false)),
|
||||||
|
"image/png" => Ok(("image/png", "png", false)),
|
||||||
|
"image/webp" => Ok(("image/webp", "webp", false)),
|
||||||
|
// infer reports HEIC/HEIF as image/heif
|
||||||
|
"image/heif" | "image/heic" => Ok(("image/heic", "heic", false)),
|
||||||
|
"video/mp4" => Ok(("video/mp4", "mp4", true)),
|
||||||
|
"video/quicktime" => Ok(("video/quicktime", "mov", true)),
|
||||||
|
"video/webm" => Ok(("video/webm", "webm", true)),
|
||||||
|
other => Err(AppError::BadRequest(format!(
|
||||||
|
"Dateityp nicht erlaubt: {other}. Erlaubt sind JPEG, PNG, WebP, HEIC, MP4, MOV, WebM."
|
||||||
|
))),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub async fn upload(
|
pub async fn upload(
|
||||||
State(state): State<AppState>,
|
State(state): State<AppState>,
|
||||||
auth: AuthUser,
|
auth: AuthUser,
|
||||||
@@ -62,8 +91,6 @@ pub async fn upload(
|
|||||||
let max_video_mb: i64 = config::get_i64(&state.pool, "max_video_size_mb", 500).await;
|
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_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 caption: Option<String> = None;
|
||||||
let mut hashtags_csv: Option<String> = None;
|
let mut hashtags_csv: Option<String> = None;
|
||||||
|
|
||||||
@@ -71,8 +98,9 @@ pub async fn upload(
|
|||||||
let name = field.name().unwrap_or_default().to_string();
|
let name = field.name().unwrap_or_default().to_string();
|
||||||
match name.as_str() {
|
match name.as_str() {
|
||||||
"file" => {
|
"file" => {
|
||||||
file_name = field.file_name().map(|s| s.to_string());
|
// The client-declared filename and Content-Type are deliberately
|
||||||
content_type = field.content_type().map(|s| s.to_string());
|
// ignored — the stored MIME and extension are derived from the
|
||||||
|
// file's magic bytes (see `classify`).
|
||||||
file_data = Some(
|
file_data = Some(
|
||||||
field.bytes().await
|
field.bytes().await
|
||||||
.map_err(|e| AppError::BadRequest(format!("Datei konnte nicht gelesen werden: {e}")))?
|
.map_err(|e| AppError::BadRequest(format!("Datei konnte nicht gelesen werden: {e}")))?
|
||||||
@@ -96,7 +124,6 @@ pub async fn upload(
|
|||||||
}
|
}
|
||||||
|
|
||||||
let data = file_data.ok_or_else(|| AppError::BadRequest("Keine Datei hochgeladen.".into()))?;
|
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;
|
let size = data.len() as i64;
|
||||||
|
|
||||||
// Validate caption length. Counted in chars (code points) to match the
|
// Validate caption length. Counted in chars (code points) to match the
|
||||||
@@ -111,25 +138,13 @@ pub async fn upload(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate file MIME type using magic bytes
|
// Derive canonical MIME + safe extension from magic bytes against a strict
|
||||||
let detected_mime = infer::get(&data);
|
// allowlist. The client Content-Type and filename are never trusted.
|
||||||
if let Some(detected) = detected_mime {
|
let (canonical_mime, safe_ext, is_video) = classify(&data)?;
|
||||||
let detected_type = detected.mime_type();
|
let canonical_mime = canonical_mime.to_string();
|
||||||
// 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)
|
// Validate file size against the per-category limit.
|
||||||
if declared_category != "application" && declared_category != detected_category {
|
let max_bytes = if is_video {
|
||||||
return Err(AppError::BadRequest(format!(
|
|
||||||
"Dateiinhalt entspricht nicht dem deklarierten Typ. Erwartet: {}, erkannt: {}",
|
|
||||||
mime, detected_type
|
|
||||||
)));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Validate file size
|
|
||||||
let max_bytes = if mime.starts_with("video/") {
|
|
||||||
max_video_mb * 1024 * 1024
|
max_video_mb * 1024 * 1024
|
||||||
} else {
|
} else {
|
||||||
max_image_mb * 1024 * 1024
|
max_image_mb * 1024 * 1024
|
||||||
@@ -141,59 +156,100 @@ pub async fn upload(
|
|||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let upload_id = Uuid::new_v4();
|
||||||
|
let event_slug = &state.config.event_slug;
|
||||||
|
let relative_path = format!("originals/{event_slug}/{upload_id}.{safe_ext}");
|
||||||
|
let absolute_path = state.config.media_path.join(&relative_path);
|
||||||
|
|
||||||
// Per-user storage quota — dynamic formula based on available disk space and the
|
// Per-user storage quota — dynamic formula based on available disk space and the
|
||||||
// number of active uploaders. Gated by master + per-area toggles so the admin can
|
// number of active uploaders. Gated by master + per-area toggles so the admin can
|
||||||
// disable it on trusted instances.
|
// disable it on trusted instances. `None` ⇒ enforcement off.
|
||||||
let quota_on = config::get_bool(&state.pool, "quota_enabled", true).await;
|
let quota_on = config::get_bool(&state.pool, "quota_enabled", true).await;
|
||||||
let storage_quota_on = config::get_bool(&state.pool, "storage_quota_enabled", true).await;
|
let storage_quota_on = config::get_bool(&state.pool, "storage_quota_enabled", true).await;
|
||||||
if quota_on && storage_quota_on {
|
let quota_limit: Option<i64> = if quota_on && storage_quota_on {
|
||||||
let estimate = compute_storage_quota(&state).await;
|
compute_storage_quota(&state).await.limit_bytes
|
||||||
if let Some(limit) = estimate.limit_bytes {
|
} else {
|
||||||
let prospective_total = user.total_upload_bytes.saturating_add(size);
|
None
|
||||||
if prospective_total > limit {
|
};
|
||||||
|
|
||||||
|
// Reserve quota and insert the row in a single transaction so the byte
|
||||||
|
// counter and the upload row are atomic (M6: no quota leak if we crash
|
||||||
|
// between them) and the reservation is a conditional UPDATE that row-locks
|
||||||
|
// (M5: concurrent uploads can no longer all pass a stale snapshot and
|
||||||
|
// overshoot the limit). The file is written before commit and unlinked on
|
||||||
|
// any rollback so a failed transaction leaves nothing behind.
|
||||||
|
let mut tx = state.pool.begin().await?;
|
||||||
|
|
||||||
|
let reserved: Option<(Uuid,)> = if let Some(limit) = quota_limit {
|
||||||
|
sqlx::query_as(
|
||||||
|
"UPDATE \"user\"
|
||||||
|
SET total_upload_bytes = total_upload_bytes + $2
|
||||||
|
WHERE id = $1 AND total_upload_bytes + $2 <= $3
|
||||||
|
RETURNING id",
|
||||||
|
)
|
||||||
|
.bind(auth.user_id)
|
||||||
|
.bind(size)
|
||||||
|
.bind(limit)
|
||||||
|
.fetch_optional(&mut *tx)
|
||||||
|
.await?
|
||||||
|
} else {
|
||||||
|
sqlx::query_as(
|
||||||
|
"UPDATE \"user\"
|
||||||
|
SET total_upload_bytes = total_upload_bytes + $2
|
||||||
|
WHERE id = $1
|
||||||
|
RETURNING id",
|
||||||
|
)
|
||||||
|
.bind(auth.user_id)
|
||||||
|
.bind(size)
|
||||||
|
.fetch_optional(&mut *tx)
|
||||||
|
.await?
|
||||||
|
};
|
||||||
|
if reserved.is_none() {
|
||||||
|
// The user exists (checked above), so a missing row here means the
|
||||||
|
// conditional limit guard rejected the reservation → over quota.
|
||||||
|
tx.rollback().await.ok();
|
||||||
return Err(AppError::TooManyRequests(
|
return Err(AppError::TooManyRequests(
|
||||||
"Du hast dein Upload-Limit für dieses Event erreicht.".into(),
|
"Du hast dein Upload-Limit für dieses Event erreicht.".into(),
|
||||||
None,
|
None,
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Determine file extension
|
// Write the file. On any failure, roll back (releasing the reservation) and
|
||||||
let ext = file_name
|
// remove a possibly-partial file.
|
||||||
.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}");
|
|
||||||
let absolute_path = state.config.media_path.join(&relative_path);
|
|
||||||
|
|
||||||
// Ensure directory exists and write file
|
|
||||||
if let Some(parent) = absolute_path.parent() {
|
if let Some(parent) = absolute_path.parent() {
|
||||||
tokio::fs::create_dir_all(parent).await.map_err(|e| AppError::Internal(e.into()))?;
|
if let Err(e) = tokio::fs::create_dir_all(parent).await {
|
||||||
|
tx.rollback().await.ok();
|
||||||
|
return Err(AppError::Internal(e.into()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let Err(e) = tokio::fs::write(&absolute_path, &data).await {
|
||||||
|
tx.rollback().await.ok();
|
||||||
|
return Err(AppError::Internal(e.into()));
|
||||||
}
|
}
|
||||||
tokio::fs::write(&absolute_path, &data).await.map_err(|e| AppError::Internal(e.into()))?;
|
|
||||||
|
|
||||||
// Update user's total upload bytes
|
let upload = match Upload::create(
|
||||||
sqlx::query("UPDATE \"user\" SET total_upload_bytes = total_upload_bytes + $2 WHERE id = $1")
|
&mut *tx,
|
||||||
.bind(auth.user_id)
|
|
||||||
.bind(size)
|
|
||||||
.execute(&state.pool)
|
|
||||||
.await?;
|
|
||||||
|
|
||||||
// Insert upload record
|
|
||||||
let upload = Upload::create(
|
|
||||||
&state.pool,
|
|
||||||
auth.event_id,
|
auth.event_id,
|
||||||
auth.user_id,
|
auth.user_id,
|
||||||
&relative_path,
|
&relative_path,
|
||||||
&mime,
|
&canonical_mime,
|
||||||
size,
|
size,
|
||||||
caption.as_deref(),
|
caption.as_deref(),
|
||||||
)
|
)
|
||||||
.await?;
|
.await
|
||||||
|
{
|
||||||
|
Ok(u) => u,
|
||||||
|
Err(e) => {
|
||||||
|
tx.rollback().await.ok();
|
||||||
|
let _ = tokio::fs::remove_file(&absolute_path).await;
|
||||||
|
return Err(e.into());
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
if let Err(e) = tx.commit().await {
|
||||||
|
let _ = tokio::fs::remove_file(&absolute_path).await;
|
||||||
|
return Err(e.into());
|
||||||
|
}
|
||||||
|
|
||||||
// Process hashtags from caption and explicit CSV
|
// Process hashtags from caption and explicit CSV
|
||||||
let mut tags: Vec<String> = Vec::new();
|
let mut tags: Vec<String> = Vec::new();
|
||||||
@@ -219,7 +275,7 @@ pub async fn upload(
|
|||||||
// Spawn compression task
|
// Spawn compression task
|
||||||
state
|
state
|
||||||
.compression
|
.compression
|
||||||
.process(upload.id, relative_path, mime.clone());
|
.process(upload.id, relative_path, canonical_mime.clone());
|
||||||
|
|
||||||
// Broadcast SSE event
|
// Broadcast SSE event
|
||||||
let dto = UploadDto {
|
let dto = UploadDto {
|
||||||
@@ -228,7 +284,7 @@ pub async fn upload(
|
|||||||
uploader_name: user.display_name,
|
uploader_name: user.display_name,
|
||||||
preview_url: None,
|
preview_url: None,
|
||||||
thumbnail_url: None,
|
thumbnail_url: None,
|
||||||
mime_type: mime,
|
mime_type: canonical_mime,
|
||||||
caption,
|
caption,
|
||||||
hashtags: tags,
|
hashtags: tags,
|
||||||
like_count: 0,
|
like_count: 0,
|
||||||
@@ -411,3 +467,41 @@ pub async fn get_original(
|
|||||||
.body(Body::from_stream(stream))
|
.body(Body::from_stream(stream))
|
||||||
.map_err(|e| AppError::Internal(e.into()))
|
.map_err(|e| AppError::Internal(e.into()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::classify;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classify_accepts_allowed_image_types() {
|
||||||
|
// PNG signature
|
||||||
|
let png = [0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0, 0, 0, 0];
|
||||||
|
let (mime, ext, is_video) = classify(&png).expect("png allowed");
|
||||||
|
assert_eq!(mime, "image/png");
|
||||||
|
assert_eq!(ext, "png");
|
||||||
|
assert!(!is_video);
|
||||||
|
|
||||||
|
// JPEG signature
|
||||||
|
let jpeg = [0xFF, 0xD8, 0xFF, 0xE0, 0, 0, 0, 0, 0, 0, 0, 0];
|
||||||
|
let (mime, ext, _) = classify(&jpeg).expect("jpeg allowed");
|
||||||
|
assert_eq!(mime, "image/jpeg");
|
||||||
|
assert_eq!(ext, "jpg");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classify_rejects_unrecognized_and_html_svg() {
|
||||||
|
// Plain HTML / script payload — infer cannot fingerprint it, so it must
|
||||||
|
// be rejected (this is the stored-XSS vector the old code let through
|
||||||
|
// via a declared `application/*` Content-Type).
|
||||||
|
let html = b"<html><script>alert(1)</script></html>";
|
||||||
|
assert!(classify(html).is_err());
|
||||||
|
|
||||||
|
// SVG (text-based) is likewise unrecognized → rejected.
|
||||||
|
let svg = b"<svg xmlns=\"http://www.w3.org/2000/svg\"><script>x</script></svg>";
|
||||||
|
assert!(classify(svg).is_err());
|
||||||
|
|
||||||
|
// Empty / garbage.
|
||||||
|
assert!(classify(&[]).is_err());
|
||||||
|
assert!(classify(&[0x00, 0x01, 0x02, 0x03]).is_err());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -36,15 +36,21 @@ pub struct UploadDto {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl Upload {
|
impl Upload {
|
||||||
pub async fn create(
|
/// Generic over the executor so it can run inside the upload transaction
|
||||||
pool: &PgPool,
|
/// (`&mut *tx`) alongside the quota-counter reservation, keeping the
|
||||||
|
/// byte-counter and the row atomic (no quota leak on a mid-write crash).
|
||||||
|
pub async fn create<'e, E>(
|
||||||
|
executor: E,
|
||||||
event_id: Uuid,
|
event_id: Uuid,
|
||||||
user_id: Uuid,
|
user_id: Uuid,
|
||||||
original_path: &str,
|
original_path: &str,
|
||||||
mime_type: &str,
|
mime_type: &str,
|
||||||
original_size_bytes: i64,
|
original_size_bytes: i64,
|
||||||
caption: Option<&str>,
|
caption: Option<&str>,
|
||||||
) -> Result<Self, sqlx::Error> {
|
) -> Result<Self, sqlx::Error>
|
||||||
|
where
|
||||||
|
E: sqlx::PgExecutor<'e>,
|
||||||
|
{
|
||||||
sqlx::query_as::<_, Self>(
|
sqlx::query_as::<_, Self>(
|
||||||
"INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, caption)
|
"INSERT INTO upload (event_id, user_id, original_path, mime_type, original_size_bytes, caption)
|
||||||
VALUES ($1, $2, $3, $4, $5, $6)
|
VALUES ($1, $2, $3, $4, $5, $6)
|
||||||
@@ -56,7 +62,7 @@ impl Upload {
|
|||||||
.bind(mime_type)
|
.bind(mime_type)
|
||||||
.bind(original_size_bytes)
|
.bind(original_size_bytes)
|
||||||
.bind(caption)
|
.bind(caption)
|
||||||
.fetch_one(pool)
|
.fetch_one(executor)
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user