Loading and saving same metadata cause errors

[qarmin]

Simplified code

fn process_file(path: &Path, dir_broken: &Path) -> Result<bool, String> {
    panic::catch_unwind(|| {
        let data = read_file_vec(path)?;
        let ext = detect_ext_from_vec(&data).ok_or_else(|| "Failed to detect file type".to_string())?;
        let metadata = load_metadata_from_vec(&data, ext)?;

        let mut data_new = data.clone();
        match metadata.write_to_vec(&mut data_new, ext){
            Ok(()) => Ok(false),
            Err(e) => {
                eprintln!("File {} is broken: {}", path.display(), e);
                let extension = path.extension().unwrap().to_string_lossy();
                fs::copy(path, dir_broken.join(format!("{}.{extension}", random::<u128>())))
                    .map_err(|e| format!("Failed to copy broken file {}: {}", path.display(), e)).unwrap();
                Ok(true)
            }
        }
    }).unwrap_or_else(|e| {
        Err(format!("Panic occurred while processing {}: {:?}", path.display(), e))
    })
}

Project - same.zip

Usage

cargo run --release -- /path_to_gather_files /path_where_save_broken_files

Reading metadata and writing back the exact same metadata to the original file should always succeed, but this errors are reported:

failed to fill whole buffer

Broken files -Broken.zip

[qarmin]

I also created fuzzer specially for this situation(which currently almost always fails) -

little_exif/fuzz/fuzz_targets/metadata.rs

Lines 43 to 53 in 88aa4a5

	fn process_metadata_strict(initial_data: &[u8], metadata: Metadata, file_extension: FileExtension) {
	let mut file_data = initial_data.to_vec();
	metadata.clone().write_to_vec(&mut file_data, file_extension).expect("Writing metadata to same buffer from which it was read should newer fail");
	let new_metadata = Metadata::new_from_vec(&file_data, file_extension).expect("Reading metadata from buffer after writing should not fail in strict mode");
	let tags_old: Vec<_> = metadata.into_iter().cloned().collect();
	let tags_new: Vec<_> = new_metadata.into_iter().cloned().collect();
	pretty_assertions::assert_eq!(tags_old, tags_new, "Metadata read from buffer after writing should be identical to the original metadata");
	}

[TechnikTobi]

Does this fix work for you?

[qarmin]

With normal files, this seems works fine - I couldn't find any images, that loading and saving metadata would fail.

I still see a problem with fuzzed files

With applied this diff(which fixes few remaining crashes) - a.txt

I was able with command

cargo run --example fuzz_test -- unpacked_files/

to get this crash, which means, that still is possible to properly load metadata from image and not being able to save this metadata to same image

thread 'main' (1388179) panicked at examples/fuzz_test.rs:97:67:
Writing metadata to same buffer from which it was read should newer fail: Custom { kind: InvalidData, error: "Mangled JPG data encountered!" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

additionally, non_strict version ignores this problem and shows different issue, that loading/saving/loading metadata generating metadata with different values

thread 'main' (1389045) panicked at examples/fuzz_test.rs:117:5:
assertion `left == right` failed: Metadata read from buffer after writing should be identical to the original metadata
  left: [Orientation([1]), XResolution([uR64 { nominator: 720000, denominator: 10000 }]), YResolution([uR64 { nominator: 720000, denominator: 10000 }]), ResolutionUnit([2]), Software("Adobe Photoshop CC 2017 (Macintosh)"), ModifyDate("2017:03:25 20:35:32"), ColorSpace([65535]), ExifImageWidth([1200]), ExifImageHeight([])]
 right: [Orientation([1]), XResolution([uR64 { nominator: 720000, denominator: 10000 }]), YResolution([uR64 { nominator: 720000, denominator: 10000 }]), ResolutionUnit([2]), Software("Adobe Photoshop CC 2017 (Macintosh)"), ModifyDate("2017:03:25 20:35:32"), ColorSpace([65535]), ExifImageWidth([1200]), ExifImageHeight([0])]
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Files - metadata.zip

[TechnikTobi]

Alright, I'll look into that.