Skip to content

MET_PerformUncompression accepts exact-size truncated streams (missing Z_STREAM_END check) #144

Description

@hjmjohnson

Summary

MET_PerformUncompression (src/metaUtils.cxx) validates the number of bytes produced but not that the zlib stream terminated with Z_STREAM_END. A truncated or trailer-corrupt stream that inflates to exactly the expected uncompressedDataSize is therefore accepted, and its trailing checksum is never verified.

This hardens the byte-count check added in #142 (which fixed the prior unconditional return true).

Root cause

  } 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;

When the decompressed data fills the output buffer exactly, the last inflate() returns Z_OK, and the next call (with avail_out == 0) returns Z_BUF_ERROR. That is < 0, so the loop exits — but Z_BUF_ERROR is deliberately excluded from the "Uncompress failed" diagnostic (it is treated as non-fatal "buffer full"). Control reaches the final check with dest_pos == uncompressedDataSize, and the function returns true even though Z_STREAM_END was never reached. The zlib trailer (Adler-32) is not validated, so a stream truncated at the size boundary passes.

Suggested fix

Require Z_STREAM_END in addition to the byte count:

  } while (err != Z_STREAM_END && err >= 0);
  inflateEnd(&d_stream);
  if (err != Z_STREAM_END)
  {
    std::cerr << "MET_PerformUncompression: compressed stream did not terminate cleanly (zlib error " << err << ")\n";
    return false;
  }
  if (dest_pos != uncompressedDataSize)
  {
    std::cerr << "MET_PerformUncompression: expected " << uncompressedDataSize << " bytes, produced " << dest_pos << '\n';
    return false;
  }
  return true;

Z_STREAM_END guarantees the whole stream (including its trailing checksum) was consumed, closing the exact-size-truncation gap while keeping the informative byte-count message for the short-read case.

Context

Found via an automated code review on ITK's re-vendor of MetaIO 8c41a1d9 (which includes #141/#142). ITK vendors MetaIO, so the fix is most useful landed here and re-vendored. Happy to open a PR if that is preferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions