BUG: Reject a mis-sized ITK_ImageOrigin series override#6632
Conversation
|
| Filename | Overview |
|---|---|
| Modules/IO/ImageBase/include/itkImageSeriesReader.hxx | Adds size validation before using origin metadata in fixed-dimension indexing. |
| Modules/IO/ImageBase/test/CMakeLists.txt | Registers the new origin-override test with the existing ImageBase test list. |
| Modules/IO/ImageBase/test/itkImageSeriesReaderOriginOverrideGTest.cxx | Adds coverage for short first and last overrides, plus a valid override case. |
Reviews (2): Last reviewed commit: "BUG: Reject a mis-sized ITK_ImageOrigin ..." | Re-trigger Greptile
position1/positionN are declared with ImageDimension entries, but ExposeMetaData assigns an ITK_ImageOrigin metadata array over them via Array::operator=, which resizes to match. A shorter override from a custom ImageIO left the arrays smaller than ImageDimension, and the next loop still indexed up to ImageDimension - 1, an out-of-bounds heap read confirmed under AddressSanitizer. Reject an override whose size does not match ImageDimension instead of silently reading past it. Addresses item B92 of InsightSoftwareConsortium#6575.
cb25b9b to
5ed08bd
Compare
|
Force-push fixes two test-quality gaps found in review, leaving the reader fix itself unchanged. What's fixed
Local validation
|
ImageSeriesReader::GenerateOutputInformation()letsArray::operator=silently shrinkposition1/positionNto the length of anITK_ImageOriginmetadata override, then still indexes them toImageDimension - 1— an out-of-bounds heap read. Addresses B92 of #6575.Root cause and fix
ExposeMetaData<Array<SpacingScalarType>>assigns throughArray::operator=, which callsSetSize()and resizes the destination to match the source. A customImageIOBasethat reports a shorterITK_ImageOriginarray leaves the local smaller thanImageDimension, and the direction-cosine loop immediately below reads past its end. Both sites now require the exposed array to be exactlyImageDimensionlong, so the invariant holds by construction for every read below.No in-tree ImageIO writes this key, so this is reachable only from an out-of-tree or custom ImageIO.
Local validation
New GoogleTest
itkImageSeriesReaderOriginOverrideGTest.cxxwith a self-contained stubImageIOBase; no files or ExternalData needed.RejectsShortOverrideOnFirstSlice/RejectsShortOverrideOnLastSlice— fail-before: throws nothing. Under AddressSanitizer the same fixture reportsheap-buffer-overflow READ of size 8atitkImageSeriesReader.hxx:180, allocated throughArray::operator=at:173. Pass-after: throws, ASan clean.AcceptsCorrectlySizedOverride— passes before and after.ctest -L ITKIOImageBase: 73/73 pass, unchanged.Behavior change: a mis-sized
ITK_ImageOriginoverride now throws fromUpdateOutputInformation()instead of reading out of bounds.AI assistance