Skip to content

Input file has corrupt header #25

Description

@ptr727

PhotoCleaner is the last stage that touches the bytes before immich-cli or immich-go uploads them, and it currently has no way to tell a structurally broken image from a good one. A real incident in this library produced 8,210 HEIC files that are byte-complete, pass every check process performs today, upload to Immich successfully, and then fail thumbnail generation forever. This issue proposes that PhotoCleaner detect that class of damage and refuse to pass it downstream.

Background

The symptom appeared in Immich as Input file has corrupt header: ... bad seek to N, where N is a few bytes past end of file. That reads exactly like truncation, and the first two investigations went that way. The files are not truncated. Every top-level box ends within the file and the iloc extents reference up to exactly the last byte.

The second suspicion was libheif, because it reports Invalid input: Missing grid images: Tiled image with 1x41=41 tiles, but only 48 tile images in file. That message is misleading twice over. It says "only 48" when 48 is more than the 41 expected, because the branch is guarded by != and the wording only fits the under-count path. More importantly the 1x41 geometry is byte-identical on every affected photo regardless of its dimensions, which is what eventually gave the game away. Per-file data does not repeat like that.

libheif is correct and Apple's originals are well formed. Do not file anything against either.

The Key Discovery

The damage was done by the iCloud sync tool (kei when it embedded XMP metadata. It re-encodes the whole ISO-BMFF container to insert the XMP packet, and the iloc (ItemLocationBox) encoder in its pinned dependency mp4-atom =0.11.0 hardcodes version 0:

// TODO: work out which version and sizes we really need for this instance.
let version = IlocVersion::V0;          // hardcoded
...
if version == IlocVersion::V1 || version == IlocVersion::V2 {
    (item_location.construction_method as u16).encode(buf)?   // unreachable
}

An iloc v0 cannot express construction_method. Apple stores the ImageGrid descriptors for grid derived items in the idat box, which is construction_method = 1, meaning "this extent offset is relative to idat". After the re-encode the box is v0, where ISO/IEC 14496-12 defines every extent as an absolute file offset. The stored offsets are still 0 and 8, so a spec-following decoder now reads file offset 0, which is the ftyp header:

00 00 00 28 66 74 79 70     "...(ftyp"
      ^^ ^^
      |  +--- 0x28 = 40 -> columns_minus_one -> 41 columns
      +------ 0x00      -> rows_minus_one    -> 1 row

1 x 41 = 41 tiles against the 48 real tiles in iref/dimg. 0x28 is simply the size of the ftyp box, which is why the number never varies.

The pixel data is untouched and the genuine descriptors are still sitting in idat, intact. Only the pointer to them is destroyed. That is the whole defect, and it is why these files are recoverable in principle.

Why the Current Pipeline Does Not Catch It

process inspects media through a single exiftool -groupNames -json call and trusts File:FileTypeExtension for the extension cross-check in ProcessTask.RenameMismatchedMimeExtensions. Nothing opens the image, decodes a frame, or walks the box tree. MediaUtilities.SupportedExtensions accepts .heic and .heif, and neither ffmpeg nor ffprobe is ever pointed at a still.

Verified against a redacted specimen of an affected file:

exiftool -groupNames -json sample.heic
  "File:FileType": "HEIC"
  "File:FileTypeExtension": "heic"
  "File:MIMEType": "image/heic"

The file is reported as a clean HEIC and sails through process with ProcessResult.Success. The same file, in the same Alpine container:

heif-info sample.heic
  Invalid input: Missing grid images: Tiled image with 1x41=41 tiles, but only 48 tile images in file
  exit=10

That is Immich's exact failure, reproduced by the library Immich actually uses. Note that exiftool also emits ExifTool:Warning: Invalid XMP on this specimen, but that is an artifact of the redaction zeroing the mdat payload and is not a detection signal.

How to Detect Affected Files

Two tiers, and I would implement the first before considering the second.

Tier 1, an in-process structural parse, no new dependency. A pure box-tree walk over a FileStream is enough to catch this and several neighboring classes of damage. This is the model already used by DateFromPath and DirectoryCleaner, it is AOT-safe, and it is unit-testable through InternalsVisibleTo without needing exiftool. The checks worth making:

  • Any item extent whose offset + length exceeds the file length. This is a generic integrity check and catches genuine truncation.
  • An iloc of version 0 in a file that also contains an idat box. idat exists to hold item data addressed relative to it, so a v0 iloc alongside it is contradictory on its face. This is the specific signature of the defect above.
  • A grid item whose descriptor, once resolved, yields rows * columns that does not equal the number of dimg references for that item. This is the semantic check, and it is the strongest of the three because it catches the damage without knowing anything about how it was caused.

A reference implementation of exactly this exists as a short Python script and correctly classifies the specimen, including recovering the true descriptors that are still present:

BROKEN  iloc v0  idat=yes  grids=[6x8=48 tiles, 4032x3024; 3x4=12 tiles, 2016x1512]  sample.heic

I can supply that script and the 3.3 KB redacted specimen as a test fixture. The specimen has every mdat payload zeroed, so it carries no image content and no personal metadata, and it still reproduces the error exactly.

Tier 2, an actual decode check. The highest-fidelity gate is the decoder Immich uses. Adding libheif-tools to Docker/Dockerfile alongside the existing exiftool and ffmpeg provides heif-info, which exits non-zero on this file. The cost is a per-file external process, so it belongs behind an opt-in option rather than in the default path. Do not substitute ffmpeg here. It does fail on the specimen, but with an unrelated diagnostic, so it is not a faithful proxy for what Immich will do.

Proposed Behavior

  • Add a validation step to the process chain in ProcessTask, next to WarnDngVersion which is the existing precedent for a non-mutating check that has an opinion about a file.
  • Put the box parsing in a new HeifValidator with internal static methods, so it is testable in isolation.
  • Add a ProcessResult.Invalid alongside Failure, with a matching counter and summary line in ProcessCommand. SkippedExtensionTracker is the precedent for aggregating a class of problem file and reporting it once at the end.
  • Make invalid files visible to the caller rather than only to the log. Today a run that logs Failed 37 files still exits 0, so a scripted pipeline cannot tell. Either a non-zero exit when invalid files are found, or a machine-readable report artifact, would let the workflow stop before the upload step.
  • Gate the upload. The README workflow chains process into immich-cli upload or immich-go upload. An invalid file should not reach that command. Moving it aside into a quarantine directory is the cleanest form of this, because it removes the file from the upload set without deleting anything, and it gives a place to retry from after a repair or a re-download.

Repair

Treat re-download from the authoritative source as the primary path. Apple holds intact originals and only the local copies were mangled. Repair is a convenience for files that can no longer be re-fetched.

A byte-length-preserving in-place patch looks feasible, and I want to flag clearly that I have not tested it. The reasoning: the stored extent offsets are still the idat-relative values, and the idat payload sits at a known absolute offset in the file. Rewriting each affected grid item's extent offset from its idat-relative value to idat_payload_offset + value produces a file that is correct under construction_method = 0, which is what a v0 iloc already declares. Because it edits offset fields in place rather than changing the box version, the box does not grow, nothing downstream of it shifts, and no other item's absolute offsets need fixing up. The alternative of rewriting the box as v1 to restore construction_method adds two bytes per item, which moves mdat and invalidates every absolute offset in the file, so it is strictly harder.

Whether libheif accepts the patched result needs to be proven before anyone relies on it. The redacted specimen is a safe subject for that experiment.

If repair is implemented, keep it out of the default path. The lesson from this incident is precisely that a tool re-encoded a container to adjust metadata and silently produced undecodable files. Any repair should be an explicit opt-in verb, should never overwrite the input in place without a backup, and should verify the result decodes before replacing anything. A validation-only default is the safe posture.

Related Detail

The upstream tool never noticed because its post-write validation only checks that the first 12 bytes still look like a HEIF ftyp brand, which passes on a mangled iloc, and its checksum verification compares against its own hash computed after the rewrite, so a corrupted file verifies as OK. Both are worth keeping in mind as anti-patterns for whatever check lands here. A magic-bytes check and a self-referential hash are not integrity checks.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions