From 9e69c95975a5be94cedec0ddfb7405e50801e0d2 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Wed, 22 Jul 2026 06:19:29 -0500 Subject: [PATCH] metaImage: validate LIST file dimension and slice count 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. --- src/metaImage.cxx | 18 ++++- src/tests/CMakeLists.txt | 1 + src/tests/testMeta13ImageList.cxx | 119 ++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 src/tests/testMeta13ImageList.cxx diff --git a/src/metaImage.cxx b/src/metaImage.cxx index 2e06dc4..be1ef54 100644 --- a/src/metaImage.cxx +++ b/src/metaImage.cxx @@ -1330,10 +1330,13 @@ MetaImage::ReadStream(int _nDims, METAIO_STREAM::ifstream * _stream, bool _readE delete[] wrds[i]; } delete[] wrds; - if ((fileImageDim == 0) || (fileImageDim > m_NDims)) + if ((fileImageDim <= 0) || (fileImageDim >= m_NDims)) { - // if optional file dimension size is not given or is larger than - // overall dimension then default to a size of m_NDims - 1. + // if optional file dimension size is not given, is not positive, or + // does not leave a dimension to iterate files over, then default to + // a size of m_NDims - 1. m_DimSize and m_SubQuantity are only + // indexable over [0, m_NDims), so out-of-range values must not reach + // the loops below. fileImageDim = m_NDims - 1; } std::string s; @@ -1346,6 +1349,7 @@ MetaImage::ReadStream(int _nDims, METAIO_STREAM::ifstream * _stream, bool _readE { totalFiles *= m_DimSize[i - 1]; } + int filesRead = 0; for (i = 0; i < totalFiles && !_stream->eof(); i++) { std::getline(*_stream, s); @@ -1382,9 +1386,17 @@ MetaImage::ReadStream(int _nDims, METAIO_STREAM::ifstream * _stream, bool _readE } readStreamTemp->close(); + filesRead++; } } delete readStreamTemp; + if (filesRead < totalFiles) + { + // The list ended early, so the tail of m_ElementData was never read. + std::cerr << "MetaImage: Read: LIST names " << filesRead << " file(s), but " << totalFiles << " are required" + << '\n'; + return false; + } } else if (m_ElementDataFileName.find('%') != std::string::npos) { diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 8f9a29c..d60c5af 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -26,3 +26,4 @@ MetaAddTest(testMeta9Landmark) MetaAddTest(testMeta10Contour) MetaAddTest(testMeta11Form) MetaAddTest(testMeta12Array) +MetaAddTest(testMeta13ImageList) diff --git a/src/tests/testMeta13ImageList.cxx b/src/tests/testMeta13ImageList.cxx new file mode 100644 index 0000000..5d73fff --- /dev/null +++ b/src/tests/testMeta13ImageList.cxx @@ -0,0 +1,119 @@ +// Regression tests for the "ElementDataFile = LIST" reader. + +#include +#include +#include +#include + +#include + +namespace +{ + +const char * const slice0 = "testMeta13_s0.raw"; +const char * const slice1 = "testMeta13_s1.raw"; + +void +WriteSlice(const char * name, unsigned char value) +{ + std::ofstream out(name, std::ios::binary); + const unsigned char buf[4] = { value, value, value, value }; + out.write(reinterpret_cast(buf), 4); +} + +// A 2x2x2 MET_UCHAR volume whose slices come from a LIST. "listBody" supplies +// the ElementDataFile value and the file names that follow it. +void +WriteHeader(const std::string & name, const std::string & listBody) +{ + std::ofstream out(name.c_str()); + out << "ObjectType = Image\n" + << "NDims = 3\n" + << "DimSize = 2 2 2\n" + << "ElementType = MET_UCHAR\n" + << "ElementSpacing = 1 1 1\n" + << "ElementByteOrderMSB = False\n" + << listBody; +} + +bool +SlicesAreOneThenTwo(MetaImage & image) +{ + for (int i = 0; i < 8; ++i) + { + const int expected = (i < 4) ? 1 : 2; + if (static_cast(image.ElementData(i)) != expected) + { + std::cout << " element " << i << " is " << image.ElementData(i) << ", expected " << expected << '\n'; + return false; + } + } + return true; +} + +} // namespace + +int +main(int, char *[]) +{ + WriteSlice(slice0, 1); + WriteSlice(slice1, 2); + + const std::string bothSlices = std::string(slice0) + "\n" + slice1 + "\n"; + + // A well-formed list reads both slices. + WriteHeader("testMeta13_valid.mhd", "ElementDataFile = LIST\n" + bothSlices); + { + MetaImage image; + if (!image.Read("testMeta13_valid.mhd")) + { + std::cout << "Well-formed LIST failed to read: FAIL" << '\n'; + return EXIT_FAILURE; + } + if (!SlicesAreOneThenTwo(image)) + { + std::cout << "Well-formed LIST read wrong values: FAIL" << '\n'; + return EXIT_FAILURE; + } + } + + // A list naming fewer files than DimSize requires must not report success, + // because the tail of the buffer is never written. + WriteHeader("testMeta13_short.mhd", std::string("ElementDataFile = LIST\n") + slice0 + "\n"); + { + MetaImage image; + if (image.Read("testMeta13_short.mhd")) + { + std::cout << "Short LIST reported success: FAIL" << '\n'; + return EXIT_FAILURE; + } + } + + // An out-of-range file dimension must fall back to NDims - 1 rather than + // indexing m_DimSize/m_SubQuantity outside [0, NDims). + const char * const outOfRange[] = { "ElementDataFile = LIST -1\n", "ElementDataFile = LIST 3\n" }; + for (const char * const listType : outOfRange) + { + WriteHeader("testMeta13_range.mhd", listType + bothSlices); + MetaImage image; + if (!image.Read("testMeta13_range.mhd")) + { + std::cout << "Out-of-range file dimension failed to read: FAIL (" << listType << ')' << '\n'; + return EXIT_FAILURE; + } + if (!SlicesAreOneThenTwo(image)) + { + std::cout << "Out-of-range file dimension read wrong values: FAIL (" << listType << ')' << '\n'; + return EXIT_FAILURE; + } + } + + std::remove(slice0); + std::remove(slice1); + std::remove("testMeta13_valid.mhd"); + std::remove("testMeta13_short.mhd"); + std::remove("testMeta13_range.mhd"); + + std::cout << "LIST element data file tests: PASS" << '\n'; + return EXIT_SUCCESS; +}