fix: cap the multipart metadata part instead of buffering it unbounded
The `metadata` JSON part in the manga-create and chapter-upload handlers was read with `Field::bytes()`, buffering the whole part before any check — a client could send a multi-hundred-MB metadata blob (up to the 200 MiB request limit) as one allocation. Image parts were already streamed with a cap; metadata was the last unbounded read. Cap it at a shared upload::MAX_METADATA_BYTES (64 KiB) via the read_capped streaming loop (413 on overflow), and remove the now-unused read_field_bytes helper so the unbounded path can't return. Tests: manga + chapter oversized-metadata parts are rejected with 413. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.128.7"
|
version = "0.128.8"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"argon2",
|
"argon2",
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "mangalord"
|
name = "mangalord"
|
||||||
version = "0.128.7"
|
version = "0.128.8"
|
||||||
edition = "2021"
|
edition = "2021"
|
||||||
default-run = "mangalord"
|
default-run = "mangalord"
|
||||||
|
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ use serde::Deserialize;
|
|||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
use uuid::Uuid;
|
use uuid::Uuid;
|
||||||
|
|
||||||
use crate::api::mangas::{next_field, read_field_bytes};
|
use crate::api::mangas::next_field;
|
||||||
use crate::api::pagination::PagedResponse;
|
use crate::api::pagination::PagedResponse;
|
||||||
use crate::app::AppState;
|
use crate::app::AppState;
|
||||||
use crate::auth::extractor::CurrentUser;
|
use crate::auth::extractor::CurrentUser;
|
||||||
@@ -107,7 +107,12 @@ async fn create(
|
|||||||
while let Some(field) = next_field(&mut multipart).await? {
|
while let Some(field) = next_field(&mut multipart).await? {
|
||||||
match field.name() {
|
match field.name() {
|
||||||
Some("metadata") => {
|
Some("metadata") => {
|
||||||
let bytes = read_field_bytes(field).await?;
|
let bytes = crate::upload::read_capped(
|
||||||
|
field,
|
||||||
|
crate::upload::MAX_METADATA_BYTES,
|
||||||
|
"metadata",
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
metadata = Some(serde_json::from_slice(&bytes).map_err(|e| {
|
metadata = Some(serde_json::from_slice(&bytes).map_err(|e| {
|
||||||
AppError::ValidationFailed {
|
AppError::ValidationFailed {
|
||||||
message: "metadata is not valid JSON".into(),
|
message: "metadata is not valid JSON".into(),
|
||||||
|
|||||||
@@ -235,7 +235,12 @@ async fn create(
|
|||||||
while let Some(field) = next_field(&mut multipart).await? {
|
while let Some(field) = next_field(&mut multipart).await? {
|
||||||
match field.name() {
|
match field.name() {
|
||||||
Some("metadata") => {
|
Some("metadata") => {
|
||||||
let bytes = read_field_bytes(field).await?;
|
let bytes = crate::upload::read_capped(
|
||||||
|
field,
|
||||||
|
crate::upload::MAX_METADATA_BYTES,
|
||||||
|
"metadata",
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
metadata = Some(parse_metadata_json(&bytes)?);
|
metadata = Some(parse_metadata_json(&bytes)?);
|
||||||
}
|
}
|
||||||
Some("cover") => {
|
Some("cover") => {
|
||||||
@@ -677,12 +682,6 @@ pub(crate) async fn next_field(
|
|||||||
.map_err(map_multipart_error)
|
.map_err(map_multipart_error)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) async fn read_field_bytes(
|
|
||||||
field: axum::extract::multipart::Field<'_>,
|
|
||||||
) -> AppResult<axum::body::Bytes> {
|
|
||||||
field.bytes().await.map_err(map_multipart_error)
|
|
||||||
}
|
|
||||||
|
|
||||||
pub(crate) fn map_multipart_error(e: axum::extract::multipart::MultipartError) -> AppError {
|
pub(crate) fn map_multipart_error(e: axum::extract::multipart::MultipartError) -> AppError {
|
||||||
let status = e.status();
|
let status = e.status();
|
||||||
if status == StatusCode::PAYLOAD_TOO_LARGE {
|
if status == StatusCode::PAYLOAD_TOO_LARGE {
|
||||||
|
|||||||
@@ -37,6 +37,15 @@ pub struct StagedImage {
|
|||||||
/// staged page out of this prefix.
|
/// staged page out of this prefix.
|
||||||
pub const STAGING_PREFIX: &str = "staging";
|
pub const STAGING_PREFIX: &str = "staging";
|
||||||
|
|
||||||
|
/// Upper bound on a multipart `metadata` JSON part, enforced as bytes arrive
|
||||||
|
/// (via [`read_capped`]). Manga/chapter metadata is title + a few short lists +
|
||||||
|
/// a description — kilobytes at most. Without this, `metadata` was the one
|
||||||
|
/// remaining part read with the unbounded `Field::bytes()`, letting a client
|
||||||
|
/// buffer up to the whole 200 MiB request body in memory as a single JSON blob
|
||||||
|
/// before any validation ran. 64 KiB is generous headroom over any legitimate
|
||||||
|
/// payload while keeping the worst case tiny.
|
||||||
|
pub const MAX_METADATA_BYTES: usize = 64 * 1024;
|
||||||
|
|
||||||
/// Read one multipart image part and write it straight to a staging key,
|
/// Read one multipart image part and write it straight to a staging key,
|
||||||
/// returning only its metadata. The per-file byte cap is enforced as bytes
|
/// returning only its metadata. The per-file byte cap is enforced as bytes
|
||||||
/// arrive, so an oversized part is rejected (413) without being fully
|
/// arrive, so an oversized part is rejected (413) without being fully
|
||||||
|
|||||||
@@ -358,3 +358,27 @@ async fn non_owner_can_upload_chapter(pool: PgPool) {
|
|||||||
assert_eq!(body["title"], "Contributed");
|
assert_eq!(body["title"], "Contributed");
|
||||||
assert_eq!(body["page_count"], 1);
|
assert_eq!(body["page_count"], 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
|
async fn chapter_upload_rejects_oversized_metadata_part(pool: PgPool) {
|
||||||
|
// The chapter `metadata` JSON part is capped as bytes arrive, just like the
|
||||||
|
// manga one, so a client can't buffer a huge JSON blob in memory. A ~200 KiB
|
||||||
|
// title blows the metadata cap before any page is staged.
|
||||||
|
let h = common::harness(pool);
|
||||||
|
let (_, cookie) = common::register_user(&h.app).await;
|
||||||
|
let manga_id = seed_manga(&h, &cookie, "Berserk").await;
|
||||||
|
|
||||||
|
let huge = "A".repeat(200 * 1024);
|
||||||
|
let resp = h
|
||||||
|
.app
|
||||||
|
.oneshot(common::post_multipart_with_cookie(
|
||||||
|
&format!("/api/v1/mangas/{manga_id}/chapters"),
|
||||||
|
MultipartBuilder::new()
|
||||||
|
.add_json("metadata", json!({ "number": 1, "title": huge }))
|
||||||
|
.add_file("page", "1.png", "image/png", &common::fake_png_bytes()),
|
||||||
|
&cookie,
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE);
|
||||||
|
}
|
||||||
|
|||||||
@@ -927,3 +927,25 @@ async fn bearer_authed_admin_cannot_edit_null_uploader(pool: PgPool) {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
#[sqlx::test(migrations = "./migrations")]
|
||||||
|
async fn create_rejects_oversized_metadata_part(pool: PgPool) {
|
||||||
|
// The metadata JSON part is capped well below the 200 MiB request limit so a
|
||||||
|
// client can't force the server to buffer a huge JSON blob in memory before
|
||||||
|
// any validation runs. A ~200 KiB description blows the metadata cap.
|
||||||
|
let h = common::harness(pool);
|
||||||
|
let (_, cookie) = common::register_user(&h.app).await;
|
||||||
|
|
||||||
|
let huge = "A".repeat(200 * 1024);
|
||||||
|
let resp = h
|
||||||
|
.app
|
||||||
|
.oneshot(common::post_multipart_with_cookie(
|
||||||
|
"/api/v1/mangas",
|
||||||
|
MultipartBuilder::new()
|
||||||
|
.add_json("metadata", json!({ "title": "Big", "description": huge })),
|
||||||
|
&cookie,
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(resp.status(), StatusCode::PAYLOAD_TOO_LARGE);
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "mangalord-frontend",
|
"name": "mangalord-frontend",
|
||||||
"version": "0.128.7",
|
"version": "0.128.8",
|
||||||
"private": true,
|
"private": true,
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
|
|||||||
Reference in New Issue
Block a user