From 7570524e5be11ef594ade46bdc2c0a8ec32f5de6 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sat, 11 Jul 2026 13:51:10 +0200 Subject: [PATCH] fix: stream cover uploads with a per-chunk size cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cover multipart path called Field::bytes(), buffering the entire field into memory before parse_image checked max_file_bytes — an oversized cover was fully allocated before rejection. Add read_capped/push_capped, which reject with 413 as soon as the running total exceeds the cap (the offending chunk is never copied in), and route both cover handlers and stage_image_part through it. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/api/mangas.rs | 6 ++- backend/src/upload/mod.rs | 78 ++++++++++++++++++++++++++++++++++----- frontend/package.json | 2 +- 5 files changed, 75 insertions(+), 15 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 91eb686..b902bd0 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.124.15" +version = "0.124.16" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index cf38a5d..8c286bc 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.124.15" +version = "0.124.16" edition = "2021" default-run = "mangalord" diff --git a/backend/src/api/mangas.rs b/backend/src/api/mangas.rs index c6a1ca8..08af8bc 100644 --- a/backend/src/api/mangas.rs +++ b/backend/src/api/mangas.rs @@ -239,7 +239,8 @@ async fn create( metadata = Some(parse_metadata_json(&bytes)?); } Some("cover") => { - let bytes = read_field_bytes(field).await?.to_vec(); + let bytes = + crate::upload::read_capped(field, state.upload.max_file_bytes, "cover").await?; cover = Some(parse_image(bytes, state.upload.max_file_bytes, "cover")?); } _ => continue, @@ -387,7 +388,8 @@ async fn put_cover( let mut cover: Option = None; while let Some(field) = next_field(&mut multipart).await? { if field.name() == Some("cover") { - let bytes = read_field_bytes(field).await?.to_vec(); + let bytes = + crate::upload::read_capped(field, state.upload.max_file_bytes, "cover").await?; cover = Some(parse_image(bytes, state.upload.max_file_bytes, "cover")?); } } diff --git a/backend/src/upload/mod.rs b/backend/src/upload/mod.rs index 163ee52..d197b9a 100644 --- a/backend/src/upload/mod.rs +++ b/backend/src/upload/mod.rs @@ -45,21 +45,13 @@ pub const STAGING_PREFIX: &str = "staging"; /// read-all-then-persist path. pub async fn stage_image_part( storage: &dyn Storage, - mut field: Field<'_>, + field: Field<'_>, upload_id: Uuid, seq: usize, max_size: usize, field_name: &str, ) -> AppResult { - let mut bytes: Vec = Vec::new(); - while let Some(chunk) = field.chunk().await.map_err(map_multipart_error)? { - if bytes.len().saturating_add(chunk.len()) > max_size { - return Err(AppError::PayloadTooLarge(format!( - "{field_name} exceeds {max_size}-byte cap" - ))); - } - bytes.extend_from_slice(&chunk); - } + let bytes = read_capped(field, max_size, field_name).await?; // Reuse the shared sniff + whitelist check; it re-verifies the size cap // (already enforced above) and derives mime/ext from magic bytes. let img = parse_image(bytes, max_size, field_name)?; @@ -73,6 +65,46 @@ pub async fn stage_image_part( }) } +/// Read one multipart part fully into memory, enforcing the per-file byte cap +/// as chunks arrive so an oversized part is rejected (413) **without being fully +/// buffered**. Use for parts whose bytes the caller needs in hand — e.g. the +/// cover image, which is written to a manga-scoped key. Page parts should prefer +/// [`stage_image_part`], which streams straight to storage. +/// +/// Replaces the `Field::bytes()` path, which buffered the entire field before +/// any size check ran, letting an attacker allocate an arbitrarily large body +/// before the cap kicked in. +pub async fn read_capped( + mut field: Field<'_>, + max_size: usize, + field_name: &str, +) -> AppResult> { + let mut bytes: Vec = Vec::new(); + while let Some(chunk) = field.chunk().await.map_err(map_multipart_error)? { + push_capped(&mut bytes, &chunk, max_size, field_name)?; + } + Ok(bytes) +} + +/// Append `chunk` to `buf`, rejecting with 413 as soon as the running total +/// would exceed `max_size` — so the oversized chunk is never copied in. Split +/// out from the read loop so the cap logic is unit-testable without a live +/// multipart field. +fn push_capped( + buf: &mut Vec, + chunk: &[u8], + max_size: usize, + field_name: &str, +) -> AppResult<()> { + if buf.len().saturating_add(chunk.len()) > max_size { + return Err(AppError::PayloadTooLarge(format!( + "{field_name} exceeds {max_size}-byte cap" + ))); + } + buf.extend_from_slice(chunk); + Ok(()) +} + pub fn parse_image(bytes: Vec, max_size: usize, field_name: &str) -> AppResult { if bytes.len() > max_size { return Err(AppError::PayloadTooLarge(format!( @@ -172,4 +204,30 @@ mod tests { assert!(matches!(err, AppError::PayloadTooLarge(_))); assert_eq!(err.code(), "payload_too_large"); } + + #[test] + fn push_capped_accumulates_under_cap() { + let mut buf = Vec::new(); + push_capped(&mut buf, b"hello ", 100, "cover").unwrap(); + push_capped(&mut buf, b"world", 100, "cover").unwrap(); + assert_eq!(buf, b"hello world"); + } + + #[test] + fn push_capped_rejects_before_copying_oversized_chunk() { + // The whole point: the offending chunk must NOT be appended — an + // oversized part is rejected without buffering it. + let mut buf = vec![0u8; 90]; + let err = push_capped(&mut buf, &[0u8; 20], 100, "cover").unwrap_err(); + assert!(matches!(err, AppError::PayloadTooLarge(_))); + assert_eq!(err.code(), "payload_too_large"); + assert_eq!(buf.len(), 90, "buffer must not grow past the cap"); + } + + #[test] + fn push_capped_allows_exactly_at_cap() { + let mut buf = vec![0u8; 90]; + push_capped(&mut buf, &[0u8; 10], 100, "cover").unwrap(); + assert_eq!(buf.len(), 100); + } } diff --git a/frontend/package.json b/frontend/package.json index 49bf457..99db7dd 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.124.15", + "version": "0.124.16", "private": true, "type": "module", "scripts": {