From 6a99edab5016887aedb84d8692dce84c06aca0a1 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Tue, 25 Aug 2026 21:37:28 +0800 Subject: [PATCH] fix(s3): reject tampered multipart payloads cleanly (#6578) --- .config/security-smoke-floor.txt | 2 +- crates/e2e_test/src/negative_sigv4_test.rs | 51 ++++++++++++++++++++++ rustfs/src/app/multipart_usecase.rs | 6 +-- rustfs/src/app/object_usecase.rs | 2 +- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/.config/security-smoke-floor.txt b/.config/security-smoke-floor.txt index 98a35fe08..b9d3f31fc 100644 --- a/.config/security-smoke-floor.txt +++ b/.config/security-smoke-floor.txt @@ -9,4 +9,4 @@ # if the selected count drops below this number, so a rename or removal that # thins the security smoke gate must update this file in the same PR. # Adding tests does not require a bump, but bumping keeps the guard tight. -16 +18 diff --git a/crates/e2e_test/src/negative_sigv4_test.rs b/crates/e2e_test/src/negative_sigv4_test.rs index 2a2a02fd8..19c3b4b0c 100644 --- a/crates/e2e_test/src/negative_sigv4_test.rs +++ b/crates/e2e_test/src/negative_sigv4_test.rs @@ -325,6 +325,57 @@ async fn tampered_payload_is_rejected() -> Result<(), Box Result<(), Box> { + init_logging(); + let mut env = RustFSTestEnvironment::new().await?; + setup(&mut env).await?; + + let key = "tampered-upload-part.bin"; + let client = env.create_s3_client(); + let upload = client.create_multipart_upload().bucket(BUCKET).key(key).send().await?; + let upload_id = upload.upload_id().ok_or("create multipart upload omitted upload_id")?; + + let path = format!("/{BUCKET}/{key}"); + let canonical_query = format!("partNumber=1&uploadId={}", urlencoding::encode(upload_id)); + let request_target = format!("{path}?{canonical_query}"); + let claimed_body = b"the-part-i-claim-to-send"; + let actual_body = b"the-part-i-really-send!!"; + assert_eq!(claimed_body.len(), actual_body.len(), "keep content-length stable for the mismatch"); + + let signer = SigV4::new(&env); + let headers = signer.sign("PUT", &path, &canonical_query, &sha256_hex(claimed_body)); + let resp = send_signed(&env, reqwest::Method::PUT, &request_target, &headers, Some(actual_body.to_vec())).await?; + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + assert_eq!( + status, + reqwest::StatusCode::BAD_REQUEST, + "multipart payload mismatch must be rejected as BadDigest, body:\n{body}" + ); + assert_error_code(&body, "BadDigest"); + + let parts = client + .list_parts() + .bucket(BUCKET) + .key(key) + .upload_id(upload_id) + .send() + .await?; + assert!(parts.parts().is_empty(), "a tampered UploadPart must not publish a part"); + + client + .abort_multipart_upload() + .bucket(BUCKET) + .key(key) + .upload_id(upload_id) + .send() + .await?; + Ok(()) +} + /// (e) A request whose `x-amz-date` is skewed beyond the server's tolerance /// (s3s default 900s / 15 min) must be rejected with RequestTimeTooSkewed / /// 403. The signature is otherwise valid: the credential-scope date and diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index 12c910ab6..89d116d14 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -71,7 +71,7 @@ use crate::app::object_data_cache::{ }; use crate::app::object_usecase::{ acquire_copy_bucket_lifecycle_locks, apply_quota_admission, build_put_like_object_lock_metadata, map_quota_check_outcome, - validate_existing_object_lock_for_write, + s3s_body_error_to_io, validate_existing_object_lock_for_write, }; use crate::app::runtime_sources::{ AppContext, current_app_context, current_object_data_cache_for_context, current_object_store_handle_for_context, @@ -988,7 +988,7 @@ impl DefaultMultipartUsecase { let mut total = 0i64; let mut buffer = bytes::BytesMut::new(); while let Some(chunk) = body_stream.next().await { - let chunk = chunk.map_err(|e| ApiError::from(StorageError::other(e.to_string())))?; + let chunk = chunk.map_err(|e| ApiError::from(s3s_body_error_to_io(e)))?; total += chunk.len() as i64; buffer.extend_from_slice(&chunk); } @@ -1022,7 +1022,7 @@ impl DefaultMultipartUsecase { let buffer_size = get_buffer_size_opt_in(size); let body = tokio::io::BufReader::with_capacity( buffer_size, - StreamReader::new(body_stream.map(|f| f.map_err(|e| std::io::Error::other(e.to_string())))), + StreamReader::new(body_stream.map(|f| f.map_err(s3s_body_error_to_io))), ); let is_disk_compressed = rustfs_utils::http::contains_key_str(&fi.user_defined, rustfs_utils::http::SUFFIX_COMPRESSION); diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index ee3e534b0..3e525c4a1 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -234,7 +234,7 @@ use crate::app::object_traffic_health::ObjectTrafficHealth; type S3StdError = Box; -fn s3s_body_error_to_io(err: StdError) -> io::Error { +pub(crate) fn s3s_body_error_to_io(err: StdError) -> io::Error { io::Error::other(err) }