Skip to content

Harden mergeTiffParts against part files missing geometry tags (SIGFPE)#1082

Open
robert-kofler42 wants to merge 1 commit into
ScrollPrize:mainfrom
robert-kofler42:harden/tiff-merge
Open

Harden mergeTiffParts against part files missing geometry tags (SIGFPE)#1082
robert-kofler42 wants to merge 1 commit into
ScrollPrize:mainfrom
robert-kofler42:harden/tiff-merge

Conversation

@robert-kofler42

Copy link
Copy Markdown

Bug

mergeTiffParts (used by the distributed vc_render_tifxyz merge step) reads the geometry/format tags from the first part file into uninitialized locals without checking the TIFFGetField return value:

uint32_t w, h, tw, th; uint16_t bps, spp, sf, comp;
TIFFGetField(first, TIFFTAG_TILEWIDTH, &tw);   // return ignored
...
uint32_t tilesX = (w + tw - 1) / tw, tilesY = (h + th - 1) / th;  // tw==0 -> SIGFPE

A part file from a killed or corrupt distributed render job can be missing those tags. In particular a stripped (non-tiled) TIFF has no TILEWIDTH/TILELENGTH, so tw/th stay uninitialized and the tile-grid computation (w + tw - 1) / tw divides by zero — a SIGFPE that takes down the whole merge.

Fix

Initialize the geometry/format locals to 0, require the four geometry tags via the TIFFGetField return value, and skip the group (count it as a failure and continue) when any are missing or zero — instead of crashing.

Verification

Adds a test_tiff_merge case that writes a stripped part TIFF and merges it:

  • unfixed: SIGFPE (signal 8, floating-point exception) — verified by running the test against the pre-fix Tiff.cpp;
  • fixed: returns false, writes nothing, no crash.

Existing happy-path merge tests still pass (3/3).

Found via a parser-hardening audit; this is the one finding in the actively-used volume-cartographer core (the others are in the vesuvius-c helper library and are being handled separately).

🤖 Generated with Claude Code

mergeTiffParts read TIFFTAG_IMAGEWIDTH/IMAGELENGTH/TILEWIDTH/TILELENGTH (and
bps/spp/sf/comp) from the first part file into uninitialized locals without
checking the TIFFGetField return value. A part file from a killed or corrupt
distributed render job can be missing those tags; in particular a stripped
(non-tiled) TIFF has no TILEWIDTH/TILELENGTH, leaving tw/th uninitialized, and
the tile grid computation `(w + tw - 1) / tw` then divides by zero — a SIGFPE
that takes down the whole merge.

Initialize the geometry/format locals to 0, require the four geometry tags via
the TIFFGetField return value, and skip the group (count it as a failure and
continue) when any are missing or zero, instead of crashing.

Adds a test_tiff_merge case that writes a stripped part TIFF and merges it: the
unfixed merge SIGFPEs (signal 8), the hardened one returns false and writes
nothing. Existing happy-path merge tests still pass.
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the scroll Team on Vercel.

A member of the Team first needs to authorize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant