Skip to content

metaImage: validate LIST file dimension and slice count#141

Merged
dzenanz merged 1 commit into
Kitware:masterfrom
hjmjohnson:fix-list-bounds-and-completeness
Jul 22, 2026
Merged

metaImage: validate LIST file dimension and slice count#141
dzenanz merged 1 commit into
Kitware:masterfrom
hjmjohnson:fix-list-bounds-and-completeness

Conversation

@hjmjohnson

Copy link
Copy Markdown
Contributor

The ElementDataFile = LIST reader accepts an out-of-range file dimension and does not check that the list named enough files. Both cases report success while leaving pixel data unread; one also indexes outside a fixed-size array.

Found while auditing MetaIO as vendored by ITK (InsightSoftwareConsortium/ITK#6575, item B64). New test testMeta13ImageList covers all three cases and fails before this change.

Out-of-range file dimension

ElementDataFile = LIST takes an optional file dimension. It was rejected only when zero or greater than NDims:

if ((fileImageDim == 0) || (fileImageDim > m_NDims))

m_DimSize and m_SubQuantity are fixed [10] members indexable over [0, NDims), so:

  • LIST -1 passes the guard, then for (i = m_NDims; i > fileImageDim; i--) totalFiles *= m_DimSize[i - 1]; walks down to i = 0 and reads m_DimSize[-1], and the slice read uses m_SubQuantity[-1]. Both are out-of-bounds reads on values taken straight from the header.
  • LIST N where N == NDims leaves totalFiles at 1 and uses the zero-initialized m_SubQuantity[NDims], so a single zero-length slice is "read" and success is returned having read nothing.

Any value outside [1, NDims) is now treated as absent and falls back to NDims - 1, which is what the existing comment already describes.

Short list

The read loop also stops at end of stream:

for (i = 0; i < totalFiles && !_stream->eof(); i++)

A list naming fewer files than DimSize requires exits early and falls through to success, leaving the tail of m_ElementData never written. Note the loop counter still reaches totalFiles in that case (the final iteration reads nothing), so the count of slices actually read is tracked separately and checked after the loop.

Testing

testMeta13ImageList builds a 2x2x2 MET_UCHAR volume from two single-slice raw files and checks four headers: a well-formed list reads both slices; a short list must fail; LIST -1 and LIST 3 must fall back and read both slices correctly.

Verified fail-before/pass-after — before the change it reports Short LIST reported success: FAIL. Full suite 13/13 passing (12 pre-existing + this one).

The same fixtures were also run through ITK's MetaImageIO, where all four previously reported success; afterwards the short list raises and the two out-of-range dimensions read correctly. ITK's Meta test suite shows no regressions.

The ReadROI variant has its own LIST branch, but it does not parse a file dimension, so only the short-list shape could apply there; it is left alone here to keep this change focused.

The ElementDataFile = LIST branch accepted an out-of-range optional file
dimension and did not verify that the list named enough files.

An explicit dimension was rejected only when zero or greater than NDims, so
a negative value reached m_DimSize[fileImageDim - 1] and
m_SubQuantity[fileImageDim], indexing outside the fixed [10] arrays, and a
value equal to NDims silently read nothing while reporting success. Treat
any value outside [1, NDims) as absent and fall back to NDims - 1, as the
existing comment already describes.

The read loop also stops at end of stream, so a list naming fewer files than
DimSize requires returned success with the tail of the buffer never written.
Count the slices actually read and fail if any are missing.
@hjmjohnson

Copy link
Copy Markdown
Contributor Author

@dzenanz Can you find someone who can merge these? After merging, I'll run the ITK UpdateFromUpstream.sh to get these fixes in place in ITK.

@dzenanz
dzenanz merged commit 225f8db into Kitware:master Jul 22, 2026
1 of 2 checks passed
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.

2 participants