From 34f808c761b850d2ca614e2e48e329f6b2c39356 Mon Sep 17 00:00:00 2001 From: Janik Besendorf Date: Tue, 15 Sep 2026 09:20:45 +0200 Subject: [PATCH] fix: recover complete files from truncated sysdiagnose archives --- src/mvt/ios/cmd_check_sysdiagnose.py | 189 +++++++++++++++++------- src/mvt/ios/modules/sysdiagnose/base.py | 32 ++-- tests/test_check_ios_sysdiagnose.py | 27 ++++ tests/test_cmd_check_sysdiagnose.py | 163 ++++++++++++++++++++ 4 files changed, 348 insertions(+), 63 deletions(-) diff --git a/src/mvt/ios/cmd_check_sysdiagnose.py b/src/mvt/ios/cmd_check_sysdiagnose.py index 26e4d49f5..d7c338201 100644 --- a/src/mvt/ios/cmd_check_sysdiagnose.py +++ b/src/mvt/ios/cmd_check_sysdiagnose.py @@ -3,6 +3,7 @@ # Use of this software is governed by the MVT License 1.1 that can be found at # https://license.mvt.re/1.1/ +import gzip import json import logging import os @@ -11,7 +12,7 @@ import tarfile import zlib from pathlib import Path, PurePosixPath -from tempfile import TemporaryDirectory +from tempfile import NamedTemporaryFile, TemporaryDirectory from typing import Any, Optional from mvt.common.command import Command @@ -23,6 +24,34 @@ log = logging.getLogger(__name__) +class _StreamingGzipFile(gzip.GzipFile): + """Deliver available bytes before reporting a missing gzip trailer. + + GzipFile.read(n) tries to fill the entire request. At a truncated end it + can raise EOFError even after decoding bytes for complete tar members. + read1() returns those bytes first and raises on the following read. The + tar stream accepts short reads, so it can retain every complete member. + """ + + def read(self, size: Optional[int] = -1) -> bytes: + return self.read1(-1 if size is None else size) + + +class _SysdiagnoseTarInfo(tarfile.TarInfo): + @classmethod + def fromtarfile(cls, archive: tarfile.TarFile) -> "_SysdiagnoseTarInfo": + try: + return super().fromtarfile(archive) + except ( + tarfile.EmptyHeaderError, # type: ignore[attr-defined] + tarfile.TruncatedHeaderError, # type: ignore[attr-defined] + tarfile.InvalidHeaderError, # type: ignore[attr-defined] + ) as exc: + # TarFile otherwise treats a short/invalid header after its first + # member as an ordinary end of archive, hiding the truncation. + raise tarfile.ReadError(str(exc)) from exc + + class CmdIOSCheckSysdiagnose(Command): def __init__( self, @@ -63,6 +92,8 @@ def __init__( self.ips_files: list[dict[str, Any]] = [] self.temp_sysdiagnose_dir: Optional[TemporaryDirectory[str]] = None self.extracted_sysdiagnose_path: Optional[str] = None + self.sysdiagnose_tar_members: list[tarfile.TarInfo] = [] + self.archive_incomplete = False @staticmethod def _parse_bugtype_header(data: bytes) -> Optional[int]: @@ -104,19 +135,40 @@ def init(self) -> None: self.log.info("Parsing sysdiagnose archive. This might take a while...") self.sysdiagnose_format = "tar" try: - self.sysdiagnose_archive = tarfile.open(self.target_path, "r:gz") - self._extract_sysdiagnose_archive() + with _StreamingGzipFile(self.target_path, "rb") as compressed: + self.sysdiagnose_archive = tarfile.open( + fileobj=compressed, mode="r|", tarinfo=_SysdiagnoseTarInfo + ) + self._extract_sysdiagnose_archive() + # Tar stops at its end marker, which can precede the gzip + # trailer. Drain the compressed stream to check its EOF/CRC. + if not self.archive_incomplete: + try: + while compressed.read1(1024 * 1024): + pass + except (EOFError, gzip.BadGzipFile, zlib.error) as exc: + self._warn_incomplete_archive(exc) except (tarfile.ReadError, EOFError, zlib.error, OSError) as exc: - # A truncated archive ends in EOFError from gzip, which Click would - # otherwise report as a bare "Aborted!" with no reason. self.log.critical( "Unable to read the sysdiagnose archive %s: %s. " "The file may be truncated or not a gzip-compressed tarball.", self.target_path, exc, ) + self.finish() sys.exit(1) + def _warn_incomplete_archive(self, exc: Exception) -> None: + self.archive_incomplete = True + self.log.warning( + "The sysdiagnose archive is truncated or damaged: %s. Recovered " + "%d complete files; incomplete files are excluded. Analysis will " + "be partial and archive integrity cannot be verified. Obtain a " + "complete copy for a full analysis.", + exc, + len(self.sysdiagnose_files), + ) + def _extract_sysdiagnose_archive(self) -> None: archive = self.sysdiagnose_archive if archive is None: @@ -124,62 +176,99 @@ def _extract_sysdiagnose_archive(self) -> None: self.temp_sysdiagnose_dir = TemporaryDirectory() extraction_root = Path(self.temp_sysdiagnose_dir.name).resolve() - archive_roots = set() - - for member in archive: - member_path = PurePosixPath(member.name.replace("\\", "/")) - if member_path.is_absolute() or ".." in member_path.parts: - self.log.warning("Skipping unsafe sysdiagnose path %r", member.name) - continue - - destination = extraction_root.joinpath(*member_path.parts).resolve() - if not destination.is_relative_to(extraction_root): - self.log.warning("Skipping unsafe sysdiagnose path %r", member.name) - continue - - if not member_path.parts: - continue - # AppleDouble sidecars (._name) carry a file's extended attributes, - # not sysdiagnose content. Device archives hold hundreds of them; - # bsdtar hides them from listings, tarfile does not. - if member_path.name.startswith("._"): - continue - archive_roots.add(member_path.parts[0]) - - if member.isdir(): - destination.mkdir(parents=True, exist_ok=True) - continue - - # Modules only need directories and regular files. Do not materialize - # links or device nodes from an untrusted sysdiagnose archive. - if not member.isfile(): - self.log.warning("Skipping unsafe sysdiagnose member %r", member.name) - continue - - normalized_name = member_path.as_posix() - self.sysdiagnose_files.append(normalized_name) - - source = archive.extractfile(member) - if source is None: - continue - - destination.parent.mkdir(parents=True, exist_ok=True) - with source, destination.open("wb") as output: - shutil.copyfileobj(source, output) - - if normalized_name.endswith(".ips"): - self._add_ips_file(str(destination), destination.read_bytes()) + archive_roots: set[str] = set() + + try: + for member in archive: + self._extract_member(archive, member, extraction_root, archive_roots) + except (tarfile.ReadError, EOFError, gzip.BadGzipFile, zlib.error) as exc: + self._warn_incomplete_archive(exc) + if not self.sysdiagnose_files: + raise tarfile.ReadError( + "No complete sysdiagnose files could be recovered" + ) from exc if len(archive_roots) != 1: + self.finish() raise ValueError("Sysdiagnose archive must contain one top-level directory") self.extracted_sysdiagnose_path = str(extraction_root / archive_roots.pop()) + def _extract_member( + self, + archive: tarfile.TarFile, + member: tarfile.TarInfo, + extraction_root: Path, + archive_roots: set[str], + ) -> None: + member_path = PurePosixPath(member.name.replace("\\", "/")) + if member_path.is_absolute() or ".." in member_path.parts: + self.log.warning("Skipping unsafe sysdiagnose path %r", member.name) + return + + destination = extraction_root.joinpath(*member_path.parts).resolve() + if not destination.is_relative_to(extraction_root): + self.log.warning("Skipping unsafe sysdiagnose path %r", member.name) + return + + if not member_path.parts: + return + # AppleDouble sidecars (._name) carry a file's extended attributes, + # not sysdiagnose content. Device archives hold hundreds of them; + # bsdtar hides them from listings, tarfile does not. + if member_path.name.startswith("._"): + return + archive_roots.add(member_path.parts[0]) + + if member.isdir(): + destination.mkdir(parents=True, exist_ok=True) + self.sysdiagnose_tar_members.append(member) + return + + # Modules only need directories and regular files. Do not materialize + # links or device nodes from an untrusted sysdiagnose archive. + if not member.isfile(): + self.log.warning("Skipping unsafe sysdiagnose member %r", member.name) + return + + normalized_name = member_path.as_posix() + + source = archive.extractfile(member) + if source is None: + return + + destination.parent.mkdir(parents=True, exist_ok=True) + # Stage each file independently. A truncated member must never be + # indexed or left for a module that scans the extracted directory. + # Staging also preserves an earlier complete entry with the same name. + with NamedTemporaryFile(dir=destination.parent, delete=False) as output: + staged_path = Path(output.name) + try: + with source: + shutil.copyfileobj(source, output) + if output.tell() != member.size: + raise tarfile.ReadError("Unexpected end of sysdiagnose member") + except BaseException: + output.close() + staged_path.unlink(missing_ok=True) + raise + try: + staged_path.replace(destination) + finally: + staged_path.unlink(missing_ok=True) + self.sysdiagnose_files.append(normalized_name) + self.sysdiagnose_tar_members.append(member) + + if normalized_name.endswith(".ips"): + self._add_ips_file(str(destination), destination.read_bytes()) + def module_init(self, module) -> None: module.ips_files = self.ips_files if self.sysdiagnose_format == "tar": if self.extracted_sysdiagnose_path is None: raise RuntimeError("Sysdiagnose archive has not been extracted") + module.sysdiagnose_tar_members = self.sysdiagnose_tar_members + module.sysdiagnose_archive_incomplete = self.archive_incomplete module.from_sysdiagnose_folder( self.extracted_sysdiagnose_path, self.sysdiagnose_files ) diff --git a/src/mvt/ios/modules/sysdiagnose/base.py b/src/mvt/ios/modules/sysdiagnose/base.py index 99187c9fb..345a67234 100644 --- a/src/mvt/ios/modules/sysdiagnose/base.py +++ b/src/mvt/ios/modules/sysdiagnose/base.py @@ -40,6 +40,10 @@ def __init__( self.tar: Optional[tarfile.TarFile] = None self.tar_files: list[str] = [] self.ips_files: list[dict[str, object]] = [] + # Metadata for safely extracted, complete archive members. None for + # directory input or commands that do not supply archive metadata. + self.sysdiagnose_tar_members: Optional[list[tarfile.TarInfo]] = None + self.sysdiagnose_archive_incomplete = False def from_sysdiagnose_folder( self, target_path: str, sysdiagnose_files: list[str] @@ -56,17 +60,21 @@ def from_sysdiagnose_tar( def _extract_timezone(self): """Determine the sysdiagnose timezone from its diagnostic log.""" file_paths = self._get_files_by_pattern("*/sysdiagnose.log") - if not file_paths: - self.log.info( - "Unable to determine the timezone in which the sysdiagnose was " - "generated. Assuming UTC for logs without a timezone." + filenames = [] + if file_paths: + content = self._get_file_content(file_paths[0]).decode( + "utf-8", errors="replace" ) - return timezone.utc - - content = self._get_file_content(file_paths[0]).decode( - "utf-8", errors="replace" - ) - filenames = re.findall(r"sysdiagnose_\S+?\.tar\.gz", content) + filenames = re.findall(r"sysdiagnose_\S+?\.tar\.gz", content) + if not filenames and self.target_path: + # A recovered archive can be missing sysdiagnose.log. Its original + # filename still records the collection offset, when not renamed. + filename = Path(self.target_path).name + if re.match( + r"sysdiagnose_\d{4}\.\d{2}\.\d{2}_\d{2}-\d{2}-\d{2}[+-]\d{4}(?:_|\.)", + filename, + ): + filenames = [filename] if not filenames: self.log.info( "Unable to determine the timezone in which the sysdiagnose was " @@ -74,9 +82,7 @@ def _extract_timezone(self): ) return timezone.utc - timestamp = "_".join( - filenames[0].removesuffix(".tar.gz").split("_")[1:3] - ) + timestamp = "_".join(filenames[0].removesuffix(".tar.gz").split("_")[1:3]) sysdiagnose_timezone = datetime.strptime( timestamp, "%Y.%m.%d_%H-%M-%S%z" ).tzinfo diff --git a/tests/test_check_ios_sysdiagnose.py b/tests/test_check_ios_sysdiagnose.py index c81e213a9..4e4c09dc3 100644 --- a/tests/test_check_ios_sysdiagnose.py +++ b/tests/test_check_ios_sysdiagnose.py @@ -1,4 +1,5 @@ import logging +import json import os import tarfile @@ -6,6 +7,8 @@ from mvt.ios.cli import check_sysdiagnose +from .test_cmd_check_sysdiagnose import _recovery_archive + CUSTOM_MODULE = """ from mvt.ios.modules.sysdiagnose import SysdiagnoseExtraction @@ -90,3 +93,27 @@ def test_check_sysdiagnose_reports_a_truncated_archive(tmp_path, caplog): assert "Unable to read the sysdiagnose archive" in caplog.text assert "truncated" in caplog.text assert "Aborted!" not in result.output + + +def test_check_sysdiagnose_recovers_and_saves_warning(tmp_path, caplog): + archive = _recovery_archive(tmp_path, "gzip-body") + module_path = tmp_path / "custom_sysdiagnose.py" + module_path.write_text(CUSTOM_MODULE, encoding="utf-8") + output_path = tmp_path / "output" + with caplog.at_level(logging.WARNING, logger="mvt"): + result = CliRunner().invoke( + check_sysdiagnose, + [ + "--load-module", + str(module_path), + "--output", + str(output_path), + str(archive), + ], + ) + assert result.exit_code == 0, result.output + assert json.loads((output_path / "custom_sysdiagnose_module.json").read_text()) == [ + {"content": "artifact"} + ] + assert "truncated or damaged" in caplog.text + assert "Recovered 3 complete files" in (output_path / "command.log").read_text() diff --git a/tests/test_cmd_check_sysdiagnose.py b/tests/test_cmd_check_sysdiagnose.py index b00b648b7..383be9d5f 100644 --- a/tests/test_cmd_check_sysdiagnose.py +++ b/tests/test_cmd_check_sysdiagnose.py @@ -1,8 +1,12 @@ import io +import gzip +import logging +import random import tarfile from datetime import timedelta from pathlib import Path +import pytest from mvt.ios.cmd_check_sysdiagnose import CmdIOSCheckSysdiagnose from mvt.ios.modules.sysdiagnose import SysdiagnoseExtraction @@ -86,6 +90,8 @@ def test_check_sysdiagnose_from_archive_closes_archive(tmp_path): ] assert command.sysdiagnose_archive is None assert "sysdiagnose/._artifact.txt" not in command.sysdiagnose_files + assert command.archive_incomplete is False + assert _test_module(command).sysdiagnose_archive_incomplete is False def test_archive_is_extracted_once_and_unsafe_members_are_skipped(tmp_path): @@ -121,3 +127,160 @@ def test_archive_is_extracted_once_and_unsafe_members_are_skipped(tmp_path): command.finish() assert not extracted_path.exists() + + +def _recovery_archive(tmp_path, damage, last_name="partial.ips"): + raw = io.BytesIO() + with tarfile.open(fileobj=raw, mode="w") as archive: + for name, data in ( + ("artifact.txt", b"artifact"), + ("sysdiagnose.log", b"sysdiagnose_2024.01.02_03-04-05+0200.tar.gz"), + ("complete.ips", b'{"bug_type": 210}\ncomplete'), + (last_name, b'{"bug_type": 210}\n' + random.Random(42).randbytes(200_000)), + ): + member = tarfile.TarInfo(f"sysdiagnose/{name}") + member.size = len(data) + member.mtime = 1_700_000_000 + member.pax_headers = {"LIBARCHIVE.creationtime": "1600000000.5"} + last_header = archive.offset + archive.addfile(member, io.BytesIO(data)) + data = gzip.compress(raw.getvalue()) + if damage == "gzip-body": + data = data[: len(data) // 2] + elif damage == "tar-body": + data = gzip.compress(raw.getvalue()[: last_header + 2000]) + elif damage == "tar-header": + data = gzip.compress(raw.getvalue()[: last_header + 100]) + elif damage == "gzip-trailer": + data = data[:-8] + elif damage == "checksum": + data = data[:-8] + bytes(x ^ 0xFF for x in data[-8:-4]) + data[-4:] + path = tmp_path / "sysdiagnose.tar.gz" + path.write_bytes(data) + return path + + +@pytest.mark.parametrize("damage", ["gzip-body", "tar-body", "tar-header"]) +def test_recovers_complete_members_and_excludes_partial_files(tmp_path, caplog, damage): + path = _recovery_archive(tmp_path, damage) + original = path.read_bytes() + command = CmdIOSCheckSysdiagnose(target_path=str(path)) + try: + with caplog.at_level(logging.WARNING): + command.init() + root = Path(command.extracted_sysdiagnose_path) + assert {p.name for p in root.iterdir()} == { + "artifact.txt", + "sysdiagnose.log", + "complete.ips", + } + assert (root / "artifact.txt").read_bytes() == b"artifact" + assert command.sysdiagnose_files == [ + "sysdiagnose/artifact.txt", + "sysdiagnose/sysdiagnose.log", + "sysdiagnose/complete.ips", + ] + assert [r["file_path"] for r in command.ips_files] == [ + str(root / "complete.ips") + ] + assert [ + m.name for m in command.sysdiagnose_tar_members + ] == command.sysdiagnose_files + assert ( + command.sysdiagnose_tar_members[0].pax_headers["LIBARCHIVE.creationtime"] + == "1600000000.5" + ) + assert command.archive_incomplete + assert "truncated or damaged" in caplog.text + assert "Recovered 3 complete files" in caplog.text + assert "Analysis will be partial" in caplog.text + assert path.read_bytes() == original + finally: + command.finish() + assert not root.exists() + + +@pytest.mark.parametrize("damage", ["gzip-trailer", "checksum"]) +def test_warns_when_tar_is_complete_but_gzip_integrity_fails(tmp_path, caplog, damage): + path = _recovery_archive(tmp_path, damage) + with caplog.at_level(logging.WARNING): + command = _run_command(path) + assert _test_module(command).results[0]["content"] == "artifact" + assert command.archive_incomplete + assert _test_module(command).sysdiagnose_archive_incomplete + assert len(command.sysdiagnose_files) == 4 + assert "archive integrity cannot be verified" in caplog.text + + +def test_truncated_duplicate_preserves_earlier_complete_file(tmp_path): + path = _recovery_archive(tmp_path, "gzip-body", last_name="artifact.txt") + command = _run_command(path) + assert _test_module(command).results[0]["content"] == "artifact" + assert command.sysdiagnose_files.count("sysdiagnose/artifact.txt") == 1 + + +def test_output_write_error_is_fatal_not_recovered(tmp_path, monkeypatch, caplog): + path = _recovery_archive(tmp_path, "none") + command = CmdIOSCheckSysdiagnose(target_path=str(path)) + + def fail_copy(*args): + raise OSError("disk full") + + monkeypatch.setattr("mvt.ios.cmd_check_sysdiagnose.shutil.copyfileobj", fail_copy) + with pytest.raises(SystemExit) as error: + command.init() + assert error.value.code == 1 + assert not command.archive_incomplete + assert "disk full" in caplog.text + assert command.temp_sysdiagnose_dir is None + assert command.sysdiagnose_archive is None + + +@pytest.mark.parametrize("offset", ["+0200", "-0530"]) +def test_recovered_timezone_falls_back_to_original_filename(tmp_path, offset): + path = _recovery_archive(tmp_path, "gzip-body") + renamed = path.with_name( + f"sysdiagnose_2026.09.14_20-04-52{offset}_iPhone-OS_iPhone_24A000.tar.gz" + ) + path.rename(renamed) + command = CmdIOSCheckSysdiagnose(target_path=str(renamed)) + try: + command.init() + module = SysdiagnoseExtraction(target_path=str(renamed)) + command.module_init(module) + # Simulate the diagnostic log being in the missing part of an archive. + module.files = [f for f in module.files if not f.endswith("/sysdiagnose.log")] + expected = ( + timedelta(hours=2) if offset == "+0200" else -timedelta(hours=5, minutes=30) + ) + assert module._extract_timezone().utcoffset(None) == expected + finally: + command.finish() + + +def test_recovery_still_skips_unsafe_paths_and_links(tmp_path): + raw = io.BytesIO() + with tarfile.open(fileobj=raw, mode="w") as archive: + for name in ("sysdiagnose/artifact.txt", "sysdiagnose/../../escaped.txt"): + member = tarfile.TarInfo(name) + member.size = 8 + archive.addfile(member, io.BytesIO(b"artifact")) + link = tarfile.TarInfo("sysdiagnose/link") + link.type = tarfile.SYMTYPE + link.linkname = "/etc/hostname" + archive.addfile(link) + path = tmp_path / "sysdiagnose.tar.gz" + path.write_bytes(gzip.compress(raw.getvalue())[:-8]) + command = CmdIOSCheckSysdiagnose(target_path=str(path)) + try: + command.init() + root = Path(command.extracted_sysdiagnose_path) + assert {p.name for p in root.iterdir()} == {"artifact.txt"} + assert command.sysdiagnose_files == ["sysdiagnose/artifact.txt"] + assert [ + m.name for m in command.sysdiagnose_tar_members + ] == command.sysdiagnose_files + assert not (root.parent.parent / "escaped.txt").exists() + assert command.archive_incomplete + finally: + command.finish()