Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DMG reader doesn't emit proper byte count for Ignore and Zero chunks #129

Open
indygreg opened this issue Jan 7, 2024 · 1 comment
Open
Labels
apple-dmg apple-dmg crate

Comments

@indygreg
Copy link
Owner

indygreg commented Jan 7, 2024

Currently, the DMG reader treats all Comment, Ignore, and Zero block chunks as an array of 0s of length compressed_bytes.

When reading DMGs produced by Apple's tooling, I was seeing corrupted partition data that coincided with the reading of Ignore and Zero block chunks. The following patch seems to fix things:

diff --git a/apple-dmg/src/lib.rs b/apple-dmg/src/lib.rs
index 6bd5678c..e0e40085 100644
--- a/apple-dmg/src/lib.rs
+++ b/apple-dmg/src/lib.rs
@@ -82,7 +82,10 @@ impl<R: Read + Seek> DmgReader<R> {
         self.r.seek(SeekFrom::Start(chunk.compressed_offset))?;
         let compressed_chunk = (&mut self.r).take(chunk.compressed_length);
         match chunk.ty().expect("unknown chunk type") {
-            ChunkType::Ignore | ChunkType::Zero | ChunkType::Comment => {
+            ChunkType::Zero | ChunkType::Ignore => {
+                Ok(Box::new(std::io::repeat(0).take(chunk.sector_count * 512)) as Box<dyn Read>)
+            }
+            ChunkType::Comment => {
                 Ok(Box::new(std::io::repeat(0).take(chunk.compressed_length)) as Box<dyn Read>)
             }
             ChunkType::Raw => Ok(Box::new(compressed_chunk)),

However, applying this patch causes tests to fail. Specifically the only_read_dmg test. This test attempts to create a copy of the vendored example.dmg file and verify it roundtrips properly. The test fails because the checksums differ:

for i in 0..dmg.plist().partitions().len() {
            let table = dmg.partition_table(i)?;
            let data = dmg.partition_data(i)?;
            let expected = u32::from(table.checksum);
            let calculated = crc32fast::hash(&data);
            assert_eq!(expected, calculated);
        }

And the checksums differ because the code change causes the emitted partition data to change, which changes the checksum.

@dvc94ch can you provide insight on how example.dmg was created? Was it created via Apple's tooling or this repo's Rust code? If it was produced with our Rust code, I think the checksums may be wrong. But without knowing what is supposed to be in the partition data, I can't (easily) pinpoint bugs in the DMG code.

@indygreg indygreg added the apple-dmg apple-dmg crate label Jan 7, 2024
@dvc94ch
Copy link
Collaborator

dvc94ch commented Jan 7, 2024

Think it was created with apple tooling, but it's been too long...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apple-dmg apple-dmg crate
Projects
None yet
Development

No branches or pull requests

2 participants