From fb649c95486aa72d46162315ed08e5184a75155f Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Wed, 22 Jul 2026 06:27:22 -0500 Subject: [PATCH] metaUtils: report uncompression failures to the caller MET_PerformUncompression always returned true, and neither MetaImage nor MetaArray looked at the result, so a truncated or corrupt stream produced a partly written buffer and a successful read. Its byte accounting also could not support a success check: dest_pos was advanced after the test that breaks out of the loop, so the output of the final inflate call -- the one reporting Z_STREAM_END -- was never counted, leaving dest_pos at 0 for a stream that decompressed in one pass. Count each call's output before acting on its status, then require that the expected number of bytes was produced. When the compressed size is absent from the header, the readers assumed the data began at the start of the file. For a single-file image that is the start of the ASCII header, which was then fed to inflate. Measure from the current position instead, which is where the element data actually begins. MetaArray::M_ReadElements also leaked its compressed buffer. --- src/metaArray.cxx | 18 ++-- src/metaImage.cxx | 18 ++-- src/metaUtils.cxx | 12 ++- src/tests/CMakeLists.txt | 1 + src/tests/testMeta14ImageCompressed.cxx | 110 ++++++++++++++++++++++++ 5 files changed, 147 insertions(+), 12 deletions(-) create mode 100644 src/tests/testMeta14ImageCompressed.cxx diff --git a/src/metaArray.cxx b/src/metaArray.cxx index 9c42c12..bcca7da 100644 --- a/src/metaArray.cxx +++ b/src/metaArray.cxx @@ -979,19 +979,27 @@ MetaArray::M_ReadElements(METAIO_STREAM::ifstream * _fstream, void * _data, int // If compressed we inflate if (m_CompressedData) { - // if m_CompressedElementDataSize is not defined we assume the size of the - // file is the size of the compressed data + // if m_CompressedElementDataSize is not defined we assume the compressed + // data runs from the current position to the end of the file if (m_CompressedElementDataSize == 0) { + const std::streampos dataPos = _fstream->tellg(); _fstream->seekg(0, std::ios::end); - m_CompressedElementDataSize = _fstream->tellg(); - _fstream->seekg(0, std::ios::beg); + m_CompressedElementDataSize = static_cast(_fstream->tellg() - dataPos); + _fstream->seekg(dataPos); } auto * compr = new unsigned char[static_cast(m_CompressedElementDataSize)]; _fstream->read(reinterpret_cast(compr), static_cast(m_CompressedElementDataSize)); - MET_PerformUncompression(compr, m_CompressedElementDataSize, static_cast(_data), readSize); + const bool uncompressed = + MET_PerformUncompression(compr, m_CompressedElementDataSize, static_cast(_data), readSize); + delete[] compr; + if (!uncompressed) + { + std::cerr << "MetaArray: M_ReadElements: could not uncompress element data" << '\n'; + return false; + } } else // if not compressed { diff --git a/src/metaImage.cxx b/src/metaImage.cxx index 2e06dc4..bc0b207 100644 --- a/src/metaImage.cxx +++ b/src/metaImage.cxx @@ -2610,15 +2610,16 @@ MetaImage::M_ReadElements(METAIO_STREAM::ifstream * _fstream, void * _data, std: // If compressed we inflate if (m_BinaryData && m_CompressedData) { - // if m_CompressedDataSize is not defined we assume the size of the - // file is the size of the compressed data + // if m_CompressedDataSize is not defined we assume the compressed data + // runs from the current position to the end of the file bool compressedDataDeterminedFromFile = false; if (m_CompressedDataSize == 0) { compressedDataDeterminedFromFile = true; + const std::streampos dataPos = _fstream->tellg(); _fstream->seekg(0, std::ios::end); - m_CompressedDataSize = _fstream->tellg(); - _fstream->seekg(0, std::ios::beg); + m_CompressedDataSize = static_cast(_fstream->tellg() - dataPos); + _fstream->seekg(dataPos); } auto * compr = new unsigned char[static_cast(m_CompressedDataSize)]; @@ -2629,7 +2630,8 @@ MetaImage::M_ReadElements(METAIO_STREAM::ifstream * _fstream, void * _data, std: return false; } - MET_PerformUncompression(compr, m_CompressedDataSize, static_cast(_data), readSize); + const bool uncompressed = + MET_PerformUncompression(compr, m_CompressedDataSize, static_cast(_data), readSize); if (compressedDataDeterminedFromFile) { @@ -2637,6 +2639,12 @@ MetaImage::M_ReadElements(METAIO_STREAM::ifstream * _fstream, void * _data, std: } delete[] compr; + + if (!uncompressed) + { + std::cerr << "MetaImage: M_ReadElements: could not uncompress element data" << '\n'; + return false; + } } else // if not compressed { diff --git a/src/metaUtils.cxx b/src/metaUtils.cxx index 1c4896b..2244c81 100644 --- a/src/metaUtils.cxx +++ b/src/metaUtils.cxx @@ -876,6 +876,10 @@ MET_PerformUncompression(const unsigned char * sourceCompressed, d_stream.next_out = uncompressedData + dest_pos; d_stream.avail_out = cur_remain_chunk; err = inflate(&d_stream, Z_NO_FLUSH); + // Account for this call's output before any exit, including the final + // call that reports Z_STREAM_END. + uInt count_uncompressed = cur_remain_chunk - d_stream.avail_out; + dest_pos += count_uncompressed; if (err == Z_STREAM_END || err < 0) { if (err != Z_STREAM_END && err != Z_BUF_ERROR) // Z_BUF_ERROR means there is still data to uncompress, @@ -884,11 +888,15 @@ MET_PerformUncompression(const unsigned char * sourceCompressed, } break; } - uInt count_uncompressed = cur_remain_chunk - d_stream.avail_out; - dest_pos += count_uncompressed; } while (d_stream.avail_out == 0); } while (err != Z_STREAM_END && err >= 0); inflateEnd(&d_stream); + if (dest_pos != uncompressedDataSize) + { + std::cerr << "MET_PerformUncompression: expected " << uncompressedDataSize << " bytes, produced " << dest_pos + << '\n'; + return false; + } return true; } diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 8f9a29c..a55c671 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -26,3 +26,4 @@ MetaAddTest(testMeta9Landmark) MetaAddTest(testMeta10Contour) MetaAddTest(testMeta11Form) MetaAddTest(testMeta12Array) +MetaAddTest(testMeta14ImageCompressed) diff --git a/src/tests/testMeta14ImageCompressed.cxx b/src/tests/testMeta14ImageCompressed.cxx new file mode 100644 index 0000000..abdfcac --- /dev/null +++ b/src/tests/testMeta14ImageCompressed.cxx @@ -0,0 +1,110 @@ +// Regression tests for reading compressed element data. + +#include +#include +#include +#include +#include + +#include + +namespace +{ + +const char * const localName = "testMeta14_local.mha"; + +// Write a compressed single-file (LOCAL) image, then strip CompressedDataSize +// from its header so the reader has to determine the size itself. +bool +WriteLocalWithoutCompressedDataSize(int quantity) +{ + { + std::vector values(static_cast(quantity)); + for (int i = 0; i < quantity; ++i) + { + values[static_cast(i)] = static_cast(i % 251); + } + MetaImage image(quantity, 1, 1, 1, MET_UCHAR, 1, values.data()); + image.CompressedData(true); + if (!image.Write(localName)) + { + return false; + } + } + + std::ifstream in(localName, std::ios::binary); + std::string contents((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + in.close(); + + const std::string key = "CompressedDataSize = "; + const size_t at = contents.find(key); + if (at == std::string::npos) + { + std::cout << " no CompressedDataSize field was written" << '\n'; + return false; + } + const size_t eol = contents.find('\n', at); + contents.erase(at, eol - at + 1); + + std::ofstream out(localName, std::ios::binary); + out.write(contents.data(), static_cast(contents.size())); + return true; +} + +} // namespace + +int +main(int, char *[]) +{ + const int quantity = 4096; + + // A LOCAL image whose header omits CompressedDataSize: the compressed data + // starts after the header, not at the start of the file. + if (!WriteLocalWithoutCompressedDataSize(quantity)) + { + std::cout << "Could not prepare the LOCAL test image: FAIL" << '\n'; + return EXIT_FAILURE; + } + { + MetaImage image; + if (!image.Read(localName)) + { + std::cout << "LOCAL image without CompressedDataSize failed to read: FAIL" << '\n'; + return EXIT_FAILURE; + } + for (int i = 0; i < quantity; ++i) + { + const int expected = i % 251; + if (static_cast(image.ElementData(i)) != expected) + { + std::cout << " element " << i << " is " << image.ElementData(i) << ", expected " << expected << '\n'; + std::cout << "LOCAL image without CompressedDataSize read wrong values: FAIL" << '\n'; + return EXIT_FAILURE; + } + } + } + + // Truncating the compressed stream must be reported, not silently accepted. + { + std::ifstream in(localName, std::ios::binary); + std::string contents((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + in.close(); + contents.resize(contents.size() - 16); + std::ofstream out("testMeta14_truncated.mha", std::ios::binary); + out.write(contents.data(), static_cast(contents.size())); + out.close(); + + MetaImage image; + if (image.Read("testMeta14_truncated.mha")) + { + std::cout << "Truncated compressed data reported success: FAIL" << '\n'; + return EXIT_FAILURE; + } + } + + std::remove(localName); + std::remove("testMeta14_truncated.mha"); + + std::cout << "Compressed element data tests: PASS" << '\n'; + return EXIT_SUCCESS; +}