diff --git a/crates/ecstore/src/store/object.rs b/crates/ecstore/src/store/object.rs index 988da280f..4d287a702 100644 --- a/crates/ecstore/src/store/object.rs +++ b/crates/ecstore/src/store/object.rs @@ -5351,13 +5351,41 @@ impl ECStore { tier_journal_api: Option>, ) -> (Vec, Vec>, Vec>) { Box::pin(async move { + let count = objects.len(); + let mut deleted = vec![DeletedObject::default(); count]; + let mut errors: Vec> = (0..count).map(|_| None).collect(); + let mut accounting = vec![None; count]; + let mut valid_objects = Vec::with_capacity(count); + let mut valid_indices = Vec::with_capacity(count); + // Validate the literal key before encoding it or taking locks. A + // dot-segment key must never reach filesystem path resolution. + for (index, object) in objects.into_iter().enumerate() { + match check_del_obj_args(bucket, &object.object_name) { + Ok(()) => { + valid_indices.push(index); + valid_objects.push(object); + } + Err(err) => errors[index] = Some(err), + } + } + if valid_objects.is_empty() { + return (deleted, errors, accounting); + } let mut opts = opts; let receipt_sink = install_tier_free_version_receipt_sink(&mut opts); - let result = self - .handle_delete_objects_with_journal_and_accounting_inner(bucket, objects, opts, tier_journal_api) + let (valid_deleted, valid_errors, valid_accounting) = self + .handle_delete_objects_with_journal_and_accounting_inner(bucket, valid_objects, opts, tier_journal_api) .await; enqueue_recorded_tier_free_versions(self, receipt_sink).await; - result + for (index, ((object, error), receipt)) in valid_indices + .into_iter() + .zip(valid_deleted.into_iter().zip(valid_errors).zip(valid_accounting)) + { + deleted[index] = object; + errors[index] = error; + accounting[index] = receipt; + } + (deleted, errors, accounting) }) .await } @@ -9094,6 +9122,58 @@ mod tests { (dirs, store) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn batch_delete_rejects_dot_segments_and_keeps_valid_result_positions() { + let bucket = "batch-delete-key-validation"; + let (_dirs, store) = multipool_version_test_store(bucket).await; + let options = ObjectOptions { + versioned: true, + ..Default::default() + }; + let victim = store + .put_object(bucket, "victim/object", &mut PutObjReader::from_vec(b"victim".to_vec()), &options) + .await + .expect("write protected-prefix control object"); + store + .put_object(bucket, "allowed/object", &mut PutObjReader::from_vec(b"allowed".to_vec()), &options) + .await + .expect("write valid deletion control object"); + let keys = [ + "allowed/../victim/object", + "allowed/object", + "a/./b", + "../other-bucket/object", + ]; + let objects = keys + .iter() + .map(|key| ObjectToDelete { + object_name: (*key).to_owned(), + ..Default::default() + }) + .collect(); + let (deleted, errors) = store.handle_delete_objects(bucket, objects, options).await; + assert_eq!(deleted.len(), keys.len()); + assert_eq!(errors.len(), keys.len()); + for index in [0, 2, 3] { + assert!( + matches!(&errors[index], Some(StorageError::ObjectNameInvalid(_, _))), + "invalid key {}: {:?}", + keys[index], + errors[index] + ); + assert!(!deleted[index].delete_marker); + } + assert!(errors[1].is_none(), "valid deletion must still succeed: {:?}", errors[1]); + assert!(deleted[1].delete_marker); + assert_eq!(deleted[1].object_name, "allowed/object"); + let after = store + .get_object_info(bucket, "victim/object", &ObjectOptions::default()) + .await + .expect("the resolved victim path must remain visible"); + assert_eq!(after.version_id, victim.version_id); + assert_eq!(after.etag, victim.etag); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn retired_marker_converges_after_bucket_recreation_on_sixteen_disks() { let bucket = "retired-marker-c11"; diff --git a/rustfs/src/app/object/delete.rs b/rustfs/src/app/object/delete.rs index 0457f6521..f231fc904 100644 --- a/rustfs/src/app/object/delete.rs +++ b/rustfs/src/app/object/delete.rs @@ -511,6 +511,15 @@ impl DefaultObjectUsecase { // cannot flood the log. let mut bulk_denial_logged = false; for (idx, obj_id) in delete.objects.iter().enumerate() { + if let Err(err) = validate_object_key(&obj_id.key, "DeleteObjects") { + delete_results[idx].error = Some(s3s::dto::Error { + code: Some(err.code().as_str().to_string()), + key: Some(obj_id.key.clone()), + message: Some(err.to_string()), + version_id: obj_id.version_id.clone(), + }); + continue; + } let raw_version_id = obj_id.version_id.clone(); let (version_id, version_uuid) = match normalize_delete_objects_version_id(raw_version_id.clone()) { Ok(parsed) => parsed, diff --git a/rustfs/src/storage/ecfs_extend.rs b/rustfs/src/storage/ecfs_extend.rs index c3807527e..d6b622c3f 100644 --- a/rustfs/src/storage/ecfs_extend.rs +++ b/rustfs/src/storage/ecfs_extend.rs @@ -340,6 +340,10 @@ pub(crate) fn validate_object_key(key: &str, operation: &str) -> S3Result<()> { )); } + if key.is_empty() || !crate::storage::storage_api::ecstore_bucket::utils::is_valid_object_prefix(key) { + return Err(S3Error::with_message(S3ErrorCode::InvalidArgument, "Invalid object key")); + } + // Log debug info for keys with special characters to help diagnose encoding issues if key.contains([' ', '+', '%']) { debug!(