validate object headers when walking a pack (PackReader.iter_headers)

src/borg/repository.py

@@ -361,20 +361,45 @@ def read(self, offset, size):
             return memoryview(self.pack_contents)[offset : offset + size]
         return self.store.load(self.key, offset=offset, size=size)
 
+    def size(self):
+        """Return the pack size in bytes; for a store-backed pack this is one metadata lookup."""
+        if self.pack_contents is not None:
+            return len(self.pack_contents)
+        return self.store.info(self.key).size
+
     def iter_headers(self):
         """Yield (chunk_id, offset, size) for each object by walking the fixed object headers.
 
         Only the headers are read, not the payloads, so locating every object costs one short
-        range read per object (or just a slice, when the pack is already in memory).
+        range read per object (or just a slice, when the pack is already in memory), plus one
+        store metadata lookup for the pack size.
+
+        Each full header must have OBJ_MAGIC and describe an object that fits into the pack,
+        otherwise the pack is corrupt and IntegrityError is raised. Ending the walk instead
+        would be worse than raising: the chunks index rebuilt from these headers would just be
+        missing the rest of the pack, and borg check --repair would then "fix" the archives by
+        dropping chunks that are there.
+        A trailing partial header is the clean end of the pack, not corruption.
         """
+        pack_hex = bin_to_hex(self.pack_id) if self.pack_id is not None else "<no id>"
+        pack_size = self.size()
         hdr_size = RepoObj.obj_header.size
         offset = 0
         while True:
             hdr_data = self.read(offset, hdr_size)
             if len(hdr_data) < hdr_size:
                 break  # clean EOF, or trailing partial bytes
             hdr = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(hdr_data))
+            if hdr.magic != OBJ_MAGIC:
+                raise IntegrityError(
+                    f'pack {pack_hex}: no object header at offset {offset} (pack corruption), run "borg check"'
+                )
             obj_size = hdr_size + hdr.meta_size + hdr.data_size
+            if offset + obj_size > pack_size:
+                raise IntegrityError(
+                    f"pack {pack_hex}: object extends past end of file at offset {offset} "
+                    f'(pack corruption), run "borg check"'
+                )
             yield hdr.chunk_id, offset, obj_size
             offset += obj_size
 

src/borg/testsuite/repository_test.py

@@ -1635,6 +1635,55 @@ def test_pack_reader_iter_headers_reads_through_store(tmp_path):
         assert list(reader.iter_headers()) == [(H(47), 0, len(obj1)), (H(48), len(obj1), len(obj2))]
 
 
+def test_pack_reader_raises_on_bad_magic():
+    # a header without OBJ_MAGIC means the walk desynced onto payload bytes: corruption, not EOF.
+    obj1 = fchunk(b"payload-one", meta=b"meta1", chunk_id=H(1))
+    obj2 = bytearray(fchunk(b"d2", meta=b"m2", chunk_id=H(2)))
+    obj2[0] ^= 0xFF  # break the magic of the second object's header
+    reader = PackReader(pack_contents=obj1 + bytes(obj2))
+    with pytest.raises(IntegrityError):
+        list(reader.iter_headers())
+
+
+def test_pack_reader_raises_on_bad_magic_through_store(tmp_path):
+    obj = bytearray(fchunk(b"FIRST", chunk_id=H(47)))
+    obj[0] ^= 0xFF
+    pack_id = H(44)
+    with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository:
+        repository.store_store("packs/" + bin_to_hex(pack_id), bytes(obj))
+        reader = PackReader(repository.store, pack_id)
+        with pytest.raises(IntegrityError):
+            list(reader.iter_headers())
+
+
+def test_pack_reader_raises_on_object_past_end_of_pack():
+    # a valid header whose declared sizes overrun the pack: the object cannot be there.
+    obj = fchunk(b"data", meta=b"meta", chunk_id=H(5))
+    pack = obj[:-1]  # drop a byte, so the header's data_size no longer fits
+    reader = PackReader(pack_contents=pack)
+    with pytest.raises(IntegrityError):
+        list(reader.iter_headers())
+
+
+def test_pack_reader_raises_on_object_past_end_of_pack_through_store(tmp_path):
+    obj = fchunk(b"FIRST", chunk_id=H(49))
+    pack_id = H(45)
+    with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository:
+        repository.store_store("packs/" + bin_to_hex(pack_id), obj[:-1])
+        reader = PackReader(repository.store, pack_id)
+        with pytest.raises(IntegrityError):
+            list(reader.iter_headers())
+
+
+def test_pack_reader_size(tmp_path):
+    obj = fchunk(b"data", meta=b"meta", chunk_id=H(6))
+    assert PackReader(pack_contents=obj).size() == len(obj)
+    pack_id = H(46)
+    with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository:
+        repository.store_store("packs/" + bin_to_hex(pack_id), obj)
+        assert PackReader(repository.store, pack_id).size() == len(obj)
+
+
 def test_pack_reader_in_memory_read_returns_view():
     # read() over an in-memory pack returns a memoryview into pack_contents.
     obj1 = fchunk(b"payload-one", meta=b"meta1", chunk_id=H(1))