Bounds-check decompression against corrupted input by ayzk · Pull Request #145 · szcompressor/SZ3

ayzk · 2026-09-01T17:25:12Z

Consolidates the eight bug-fix pull requests it replaces (#131 #132 #133 #134 #135 #137 #138 #139)
with what reviewing them turned up.

Six commits, each meant to be read on its own:

	Commit	What it is
1	`Include what each header uses`	`#include`/whitespace/comment churn only — skim it
2	`Bounds-check decompression against corrupted input`	the eight PRs, and everything reviewing them turned up
3	`Close what the first pass left open in the decompression path`	a pass over commit 2 for necessity rather than correctness
4	`Fix what the HDF5 filter and the fuzzer found`	what CI and a wider corruption sweep caught afterwards
5	`Give the lossless layer a capacity in and a length out`	the second of the two interface changes below
6	`Split the integration datasets that are too slow to be one job`	CI only, no library code

Version 3.3.2 → 3.3.3. SZ3_DATA_VERSION stays at 3.3.2, because the compressed format is
unchanged: a stream written by this build is byte-identical to one written by master, and
either build reads the other's output. Verified on interp / interp_lorenzo / lorenzo_reg / nopred /
lossless / biomd, serial and OpenMP, on hurricane, miranda and nyx.

Two module interfaces do change, detailed in the next section. Config::load gains a parameter
too, but it is a concrete class, not one of the concepts:: interfaces a module implements. No
bytes moved.

Supersedes #143.

The two module interfaces that change

Everything else in this branch is source-compatible. These two are not: a module written
against 3.3.2 needs the corresponding edit. Nothing else in concepts:: moves —
DecompositionInterface, QuantizerInterface and CompressorInterface are untouched.

concepts::EncoderInterface::decode — takes how many bytes it may read, and debits what it
consumes:

-    virtual std::vector<T, TAllocator> decode(const uchar *&bytes, size_t targetLength) = 0;
+    virtual std::vector<T, TAllocator> decode(const uchar *&bytes, size_t targetLength, size_t &remaining_length) = 0;

An encoder could not previously tell how many bytes it was allowed to touch: HuffmanEncoder
read its payload length out of the stream and walked that far, BypassEncoder memcpy'd
sizeof(T) * targetLength, RunlengthEncoder read a value and a count per run. targetLength
still says when to stop producing — an entropy-coded stream has no terminator, so neither
number substitutes for the other, and both are available at every call site.

concepts::LosslessInterface::decompress — takes a capacity rather than an in-out length,
and returns the length:

-    virtual size_t decompress(const uchar *src, const size_t srcLen, uchar *&dst, size_t &dstLen) = 0;
+    virtual size_t decompress(const uchar *src, size_t srcLen, uchar *&dst, size_t dstCap) = 0;

One size_t& meant the capacity going in and the decompressed length coming out. A caller with
no buffer had nothing to put there, so zero came to mean "no cap" — and a caller that did own a
buffer but passed zero got an unbounded write, which is how ALGO_LOSSLESS overran its output.
A null dst still asks the callee to allocate. This is the shape compress already had:

size_t compress  (const uchar *src, size_t srcLen, uchar *dst,  size_t dstCap);
size_t decompress(const uchar *src, size_t srcLen, uchar *&dst, size_t dstCap);

The pull requests it replaces

PR	Defect	Taken
#131	`Config::load` computed the end of the config blob as `c + confSize` after `c` had passed the length prefix — one byte too far, so the last optional-field guard read a field that is not there, past the end of the stream.	yes
#132	Bounds-checks the decompression path: the header in `SZ_decompress` (which ignored the `cmpSize` it was given), `Config::load`, `Lossless_zstd`, Huffman tree loading, `unpad_tree` child indices, predictor selection indices, `recover_unpred`, `unpred_size` before `resize`, OMP thread counts and per-thread sizes, and buffer ownership on throwing paths.	yes, except the format change — see below
#133	`out1 << (64 - len)` with `len == 0` — a single-symbol Huffman tree, which a constant field produces — shifts a 64-bit value by 64.	yes
#134	`HuffmanEncoderV2::loadAsDFSOrder` read its header and DFS bitstream with no bounds, `reserve(tree.n << 1)` overflowed, and it advanced the cursor without debiting `remaining_length`.	yes, plus two gaps closed below
#135	`smallIdx`, read from the stream, indexed the fixed-size `magicInts` table unchecked; `XtcBasedEncoder`'s first `malloc` was overwritten and leaked; `Lossless_bypass` did not check its `malloc`.	yes, in a two-sided form — see below
#137	`RegressionPredictor::load` debited `remaining_length` by the decoded bin count while `decode()` advances by the encoded byte count; `ComposedPredictor::load` never debited it at all.	yes, now carried by the `decode` signature itself
#138	`static_cast<int64_t>(fabs(diff) * error_bound_reciprocal)` is undefined for NaN, infinities and huge magnitudes.	yes
#139	Scratch buffers were `free()`d at the end of a function that can throw; the OMP path's per-chunk capacity omitted the size header `Lossless_zstd::compress` writes, so a poorly compressible chunk threw out of a parallel region.	yes

Where this departs from the PRs as written

decode takes the byte budget instead of a setter, as described above. #132 instead moved the
count field in the stream; that is a format change, and SZ3_DATA_VERSION was not bumped, so a
file from any released SZ3 fails with a misleading invalid node count.

Config::load follows the tree's own convention. It takes size_t &remaining_length and reads
through the bounded read() overload, like every other load() here. The HDF5 filter therefore
passes cd_nelmts rather than reading unbounded — with a default Config's size_est() sizing the
buffer, an ordinary EB_ABS_AND_REL dataset had 43 bytes of config read out of 36.

XtcBasedEncoder's magicInts lookups are clamped on both sides, not rejected on one. LASTIDX
is the table's length, and the encoder walks to it whenever no entry reaches minDiff — every input
with fewer than two triplets, minDiff being still INT_MAX. Both sides read one past the table
there; rejecting it on decode alone broke ALGO_BIOMDXTC for inputs under six elements.

#132's bound on the internal decompression buffer is dropped. That buffer is sized from the bin
count and type, which no bound derivable from conf covers.

Found while reviewing, fixed here

ALGO_LOSSLESS wrote past the output buffer. The declared size went to ZSTD_decompress as the
capacity of a buffer the caller owns; the size check that followed ran after the write. Reachable
through the public API. Lossless_zstd now honours a caller's capacity.
Two signed overflows on values taken from the stream, both undefined and both pre-existing: the
doubled state count in HuffmanEncoder::load, and the doubled index in LinearQuantizer::recover_pred.
Config::save and load disagreed about unknown error bound modes. save always writes a bound;
load's if-else chain had no else, so a mode above 5 left those 8 bytes unread and shifted every
field below — dataType, which the HDF5 filter switches on, included.
ALGO_BIOMDXTC wrote uninitialised heap into the compressed file. Its bit-packing buffer was
malloc'd, so the same input did not compress to the same bytes twice.
MDZ passed its buffer capacity to decompress() as the stream length, having discarded what
compress() returned. zstd rejected every frame and the result was decoded from uninitialised
memory, which nothing noticed because the smoke test only prints a ratio.
HuffmanEncoderV2 sized its dense tables with maxval read straight from the stream; tree.n is an
int holding a 64-bit read, so its bound is checked with an explicit sign test.
test_lossless.cpp passed an uninitialised size to decompress().
The suite never exercised the HDF5 filter's buffer sizing. test_h5_filter.py gains a small-chunk
mode, capped at 4096 chunks — over a whole 280M-element field it would mean 35M chunks and take the
process past a runner's memory.

The third commit — a necessity pass over the second

Every item was reproduced before it was changed.

The Huffman node pool was sized from the stream's state count, bounded only from below.
createHuffmanTree allocates ~200 bytes per unit, so a hundred-byte file reached 5.7 GB. Decoding
never touches the code tables that count sizes, so the pool comes from nodeCount, which the buffer
already bounds — and the state count, its range check and the pool-capacity check all go away.
unpad_tree allowed two parents to name one child. Strictly increasing indices rule out a cycle
but not that: a 190-byte stream with nodeCount 43 built two million nodes. Each index is taken once.
The declared bin count is capped at conf.num, and each grid-walking decomposition checks its
own floor. ALGO_NOPRED and the lorenzo half of the default algorithm walked off an empty vector.
conf.num is a ceiling only — SZBioMDXtcDecomposition legitimately emits fewer.
decode's byte budget was subtracted unchecked in three encoders, so an over-read wrapped it to
near SIZE_MAX. ArithmeticEncoder also ran past the end of every stream its own encoder wrote.
Introduced by commit 2 and fixed here: XtcBasedEncoder leaked and double-freed, Lossless_bypass
checked capacity only where an overrun is impossible, ten headers threw without <stdexcept>.
InterpolationDecomposition::load guarded two of five stream-read fields with assert, which a
release build removes. The sz3 CLI sized its output buffer below what SZ_compress demands, so
sz3 -f -i small.f32 -3 8 8 8 -M ABS 1e-3 aborted.
SZBioMDXtcDecomposition wrote two uninitialised members into the stream, so a 1-D ALGO_BIOMDXTC
compression produced a different file every run.

The fourth commit — what CI and a wider sweep caught

The HDF5 filter stopped working. Commit 2's Config::load required the declared config size to
match what the buffer holds; cdvalueHelper.py declares 40 bytes and writes 32 (the two commented-out
fields above it). The size is now a stopping point clamped to the buffer, and a config with no
dimensions is legal — the filter takes one from cd_values and fills in the shape from the dataset
in set_local. Reproduced against a real HDF5 build; a library with this fix still reads the old
helper's cd_values.
bytes2vector shifted in int. Pre-existing, and not only a fuzzing artifact: any dimension past
2³² is silently truncated on load — 2⁴⁰ comes back as 256.
The Huffman tree arrays leaked whenever the tree turned out to be malformed. They are vectors now,
which also deletes the four malloc/memset/free triples in each of the three branches.
Huffman decode tests count where a symbol is produced rather than once per bit, which is what
master did. Commit 2's extra per-bit comparison cost ALGO_NOPRED 6% of its decompression speed.

Verification

Format: byte-identical to master, both directions, across the algorithms and datasets above.
Sanitizers (Linux, gcc 13.3): on a 3401-case corruption sweep, master dies with a SEGV in
BlockwiseDecomposition; this branch reports 0 ASan and 0 UBSan errors and handles every case.
Real data through five algorithms under ASan+UBSan is clean.
Corruption fuzzing: master crashes 413 / 346 / 320 / 1993 times for lorenzo_reg / interp /
interp_lorenzo / nopred over ~150k mutated streams each; this branch, 0.
HDF5: end-to-end through the filter (write, close, reopen, read) for EB_ABS, EB_ABS_AND_REL
and EB_ABS_OR_REL.
CI: hacc ran 2h09 as a single integration job, scale-letkf 1h00 and exaalt-helium 46m,
against under half an hour for everything else. A matrix entry may now name the fields to run, so
those three are spread over several jobs; no job is much past 35 minutes and no data is dropped.
Throughput: unchanged. 5 fields × 4 algorithms × 3 bounds on x86_64/gcc 13.3, pinned, best of
many — compression 0.968–1.007, decompression 0.982–1.018 of master, compression ratio identical
at every point. Measured with matched inlining: GCC's default inline-unit-growth happens to inline
the Huffman frequency map in one tree and not the other, which moves compression by up to 8% in
either direction and has nothing to do with any change here.

🤖 Generated with Claude Code

These headers took the standard library names they use from whatever included them first, so they did not compile on their own and broke under a different standard library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Consolidates the eight open bug-fix pull requests (#131 #132 #133 #134 #135 #137 #138 #139) with the findings from reviewing them, as one change. The compressed stream is untrusted, and several modules read lengths and counts out of it and then used them to index, allocate or loop without checking them against anything. Version 3.3.2 -> 3.3.3. SZ3_DATA_VERSION stays at 3.3.2: the format is unchanged. A stream written by this build is byte-identical to one written by master, and either build reads the other's output; verified on interp/lorenzo_reg/nopred in 1D, 2D and 3D. Encoder::decode now takes the remaining byte count ------------------------------------------------ Its signature carried the symbol count and nothing else, so an encoder could not tell how many bytes it was allowed to touch. HuffmanEncoder read its payload length out of the stream and walked that far; BypassEncoder memcpy'd sizeof(T) * targetLength; RunlengthEncoder read a value and a count per run. None of them had anything to compare against. decode() now takes `size_t &remaining_length` alongside targetLength and charges what it consumed, the same shape load() already had. The two numbers are independent -- an entropy coder's bitstream has no terminator, so the symbol count is what says stop, while the byte count is what says how far it may read -- and both are available at every call site. The symbol count stays where it is in the stream, so no bytes moved. Other bounds ------------ - HuffmanEncoderV2's tree loading, and XtcBasedEncoder, against corrupted input - ComposedPredictor's predictor selection index, and its value - RegressionPredictor's coefficient stream, which each block consumes N + 1 entries of - InterpolationDecomposition's stored dimensions, which drive a grid walk over buffers that conf sizes - the declared bin count, against the element count conf carries - the bins InterpolationDecomposition and TimeSeriesDecomposition walk, checked once before the walk rather than on each access - Config::load reading one byte past the config. It now takes `size_t &remaining_length` and reads through the bounded overload, like every other load() in the tree, so the HDF5 filter passes cd_nelmts rather than reading unbounded. Both save() and load() reject an error bound mode with no branch: save() would write a blob load() cannot parse, and load() would leave the bound unread and shift every field below it, dataType included - the HDF5 filter's compressed buffer, sized from SZ_compress_size_bound - ALGO_LOSSLESS's output buffer: the declared size went to ZSTD_decompress as the capacity of a buffer the caller owns, so a stream declaring more than conf.num elements wrote past it. The size check that followed ran after the write. Lossless_zstd now honours a caller's capacity Also ---- - Huffman's shift for single-symbol input, and the non-finite float cast in LinearQuantizer, were undefined behaviour; so were two signed overflows on values taken from the stream, the doubled state count in HuffmanEncoder::load and the doubled index in LinearQuantizer::recover_pred - scratch buffers are held as unique_ptr so an exception from the encoder or the lossless layer does not leak them - PR #132's bound on the internal decompression buffer is dropped: that buffer is sized from the bin count and type, which no bound derivable from conf alone covers XtcBasedEncoder's magicInts lookups are clamped on both sides rather than rejected on one. LASTIDX is the table's length, and the encoder walks to it whenever no entry reaches minDiff -- which is every input with fewer than two triplets, since minDiff is then still INT_MAX. Both sides read one past the table there; rejecting it on decode alone broke ALGO_BIOMDXTC for inputs under six elements. Its bit-packing buffer is also zeroed: it went into the compressed output uninitialised, which is why the same input did not compress to the same bytes twice. MDZ passed its buffer capacity to decompress() as the stream length, having discarded what compress() returned. zstd rejected every frame, and the result was decoded from uninitialised memory without anything noticing. Not taken from the PRs as written: #132's internal-buffer bound (above). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A review of the previous commit for necessity rather than correctness. Every item below was reproduced before it was changed; the compressed format is still byte-identical to master's. Bounded what the first pass only re-shaped ------------------------------------------ HuffmanEncoder::load sized its node pool from the state count stored in the stream, which it bounded only from below. createHuffmanTree mallocs and memsets about 200 bytes per unit, so a hundred-byte file reached 5.7 GB, and the ceiling the check permitted was hundreds of gigabytes. Decoding never touches the code tables that count sizes, so the pool now comes from nodeCount, which the buffer already bounds -- and the state count, its range check and the pool-capacity check all go away. unpad_tree's index check made indices strictly increasing, which rules out a cycle but not two parents naming one child: a 190-byte stream with nodeCount 43 built two million nodes, and 255 would not finish. Each index is now taken once, which is what the pool size assumes. The bin count a stream declares is capped at conf.num in SZGenericCompressor, and each decomposition that walks a grid checks it has enough. ALGO_NOPRED and ALGO_LORENZO_REG -- the lorenzo half of the default algorithm -- walked off the end of an empty vector, and BlockwiseDecomposition took &quant_inds[0] before looking. conf.num is only a ceiling: SZBioMDXtcDecomposition's multi-frame path stops at the first fill frame and legitimately emits fewer. decode()'s new byte budget was subtracted unchecked in ArithmeticEncoder, HuffmanEncoderV2 and XtcBasedEncoder, so an over-read wrapped it to near SIZE_MAX and unbounded every later read. They now charge through a helper that refuses to charge more than is left. ArithmeticEncoder's decoder also opens with an eight-byte read and keeps its cursor ahead of what it consumed, so it ran past the end of every stream its own encoder wrote; the encoder pads by that much. Fixed, having been introduced by the previous commit ---------------------------------------------------- XtcBasedEncoder::encode leaked both of its buffers, and decode had two throws between its allocations and their free. Both now hold them in unique_ptr. Lossless_bypass::decompress checked the caller's capacity only where it allocates the buffer itself -- the path where an overrun is impossible -- and then memcpy'd srcLen bytes regardless. Ten headers threw std::out_of_range without including <stdexcept>, in a branch whose first commit is about headers including what they use. InterpolationDecomposition::load reads five fields from the payload and only the dimensions were checked. interp_id and direction_sequence_id index fixed tables, and block size and anchor stride were guarded by asserts, which a release build compiles out. All four are checked in init() now, where the permutation table they index is built. The sz3 CLI sized its output buffer at twice the raw size, which is below what SZ_compress demands for a small input -- the same sizing the HDF5 filter had. `sz3 -f -i small.f32 -3 8 8 8 -M ABS 1e-3` aborted; it uses SZ_compress_size_bound now. Three places wrote a bound that MemoryUtil's read() overload already applies -- HuffmanEncoder's payload length, RunlengthEncoder's per-run pair -- and one compared sizeof against zero, which it never is. Also ---- SZBioMDXtcDecomposition's firstFillFrame_ and fillValue_ had no initialiser and are written to the stream unconditionally, so a 1-D ALGO_BIOMDXTC compression produced a different file every run. EncoderInterface::decode's documentation covers the parameter it gained, and .gitignore no longer carries entries for directories this branch does not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Config::load rejected two things it should accept. The declared config size is now a stopping point clamped to the buffer rather than a requirement, and a config with no dimensions is legal: the HDF5 filter takes one from cd_values and fills in the shape from the dataset. Every read is bounds-checked either way. cdvalueHelper.py declared 40 bytes while writing 32. Corrupting compressed streams four ways across five algorithms turned up two more: the Huffman tree arrays leaked when the tree turned out to be malformed, and bytes2vector shifted in int, which silently truncates any dimension past 2^32. Huffman decode tests `count` where a symbol is produced instead of once per bit, which is what master did; the extra comparison cost NOPRED 6% of its decompression speed. Two checks from the previous commit assumed more than they could. The anchor stride is a size_t, so the -1 that means "unset" arrives as SIZE_MAX and is normalised to zero a few lines further down; the check belongs after that, where the value is either zero or a stride. And the bin count is not bounded by the element count: a block transform emits a padded block per partial block and legitimately exceeds it. Each decomposition that walks a grid still checks it has enough. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

decompress() took one size_t by reference that meant the capacity going in and the decompressed length coming out. A caller with no buffer had nothing to put there, so zero came to mean "no cap" -- and a caller that did own a buffer but passed zero got an unbounded write, which is how ALGO_LOSSLESS overran its output. The parameter is now dstCap, by value, capacity only; the length is the return value, the shape compress() already had. A null dst still asks the callee to allocate, and the capacity is checked exactly when the caller owns the buffer, so nothing keys on zero. Both implementations decide ownership once rather than testing dst twice, and zstd reports a frame that errors and one that produces the wrong length the same way, since both mean the stream does not decompress to what it declares. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ayzk · 2026-09-16T17:25:07Z

@robertu94
This update will change two SZ3 interface APIs, is it fine from your side? The data format is untouched.

concepts::EncoderInterface::decode — takes how many bytes it may read, and debits what it
consumes:

-    virtual std::vector<T, TAllocator> decode(const uchar *&bytes, size_t targetLength) = 0;
+    virtual std::vector<T, TAllocator> decode(const uchar *&bytes, size_t targetLength, size_t &remaining_length) = 0;

concepts::LosslessInterface::decompress — takes a capacity rather than an in-out length,
and returns the length:

-    virtual size_t decompress(const uchar *src, const size_t srcLen, uchar *&dst, size_t &dstLen) = 0;
+    virtual size_t decompress(const uchar *src, size_t srcLen, uchar *&dst, size_t dstCap) = 0;

hacc ran 2h09 as a single job, scale-letkf 1h00 and exaalt-helium 46m, against under half an hour for everything else. A matrix entry may now name the fields to run, so those three are spread over several jobs and none is much past half an hour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The six commits of master-fixes (#145), squashed: decompression is bounds-checked against corrupted input, several bugs reachable on valid data are fixed, and two module interfaces change — EncoderInterface::decode takes the byte budget it may read, LosslessInterface::decompress takes a capacity rather than an in-out length. fz's own modules are carried onto the new signatures: the five encoders it has that master does not, and the decompositions whose output type it generalised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

#145's content was already here through the squash in 160269d, so this brings #127's HDF5_IS_PARALLEL propagation and the rest of #140's ALWAYS_INLINE removals. Every file both branches have now matches master's count; what is left sits in quantizers master does not have. The conflicts were fz's additions against master's include reordering. fz keeps its doc comments, the four extra algorithm wirings, the bin_type work and the SVD config fields; master's ordering and its QuantOptimization dedup are taken. The merge itself stacked three copies of one compress() doc block and re-added two duplicate includes, removed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Unresolved decoder bounds, stream-accounting, allocation, and compatibility issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR hardens SZ3 decompression against corrupted input while preserving the compressed format and updating related interfaces, tests, HDF5 integration, and CI.

Changes:

Adds bounded decoding, validation, overflow protections, and safer ownership.
Updates encoder and lossless interfaces and their callers.
Expands HDF5 coverage and partitions long-running integration jobs.

File summaries

File	Summary
`tools/test/modules/test_lossless.cpp`	Updates lossless API usage.
`tools/test/modules/test_encoder.cpp`	Passes decoder byte budgets.
`tools/test/integration/test_h5_filter.py`	Adds small-chunk HDF5 coverage.
`tools/test/integration/integration_test_driver.py`	Supports field-level dataset selection.
`tools/test/deprecated/SZBlockInterpolationCompressor.hpp`	Updates deprecated lossless usage.
`tools/sz3/sz3.cpp`	Uses the calculated compression bound.
`tools/mdz/include/mdz.hpp`	Preserves compressed stream lengths.
`tools/H5Z-SZ3/test/cdvalueHelper.py`	Corrects serialized config sizing.
`tools/H5Z-SZ3/test/cdvalueHelper.cpp`	Bounds config loading.
`tools/H5Z-SZ3/src/H5Z_SZ3.cpp`	Bounds HDF5 config and output buffers.
`README.md`	Documents version 3.3.3.
`include/SZ3/utils/Statistic.hpp`	Adds direct dependencies.
`include/SZ3/utils/Sample.hpp`	Adds required headers.
`include/SZ3/utils/QuantOptimization.hpp`	Adds required headers.
`include/SZ3/utils/MemoryUtil.hpp`	Converts unchecked reads to errors.
`include/SZ3/utils/KmeansUtil.hpp`	Fixes sample allocation and indexing.
`include/SZ3/utils/Iterator.hpp`	Adds required dependencies.
`include/SZ3/utils/Extraction.hpp`	Adds direct dependencies.
`include/SZ3/utils/Config.hpp`	Validates serialized configurations.
`include/SZ3/utils/ByteUtil.hpp`	Fixes integer-width bit shifting.
`include/SZ3/utils/BlockwiseIterator.hpp`	Adds unpadded value materialization.
`include/SZ3/quantizer/Quantizer.hpp`	Adds standard headers.
`include/SZ3/quantizer/LinearQuantizer.hpp`	Hardens quantization and recovery.
`include/SZ3/preprocessor/Transpose.hpp`	Adds required dependencies.
`include/SZ3/preprocessor/PreFilter.hpp`	Adds required dependencies.
`include/SZ3/predictor/RegressionPredictor.hpp`	Bounds coefficient decoding.
`include/SZ3/predictor/LorenzoPredictor.hpp`	Fixes pointer arithmetic.
`include/SZ3/predictor/ComposedPredictor.hpp`	Validates predictor selections.
`include/SZ3/lossless/Lossless.hpp`	Changes lossless capacity semantics.
`include/SZ3/lossless/Lossless_zstd.hpp`	Enforces decompression capacity.
`include/SZ3/lossless/Lossless_bypass.hpp`	Adds capacity and allocation checks.
`include/SZ3/encoder/XtcBasedEncoder.hpp`	Adds allocation and decode validation.
`include/SZ3/encoder/RunlengthEncoder.hpp`	Validates run-length decoding.
`include/SZ3/encoder/HuffmanEncoderV2.hpp`	Bounds tree and stream decoding.
`include/SZ3/encoder/HuffmanEncoder.hpp`	Hardens tree reconstruction.
`include/SZ3/encoder/Encoder.hpp`	Changes the decoder interface.
`include/SZ3/encoder/BypassEncoder.hpp`	Bounds raw decoding.
`include/SZ3/encoder/ArithmeticEncoder.hpp`	Adds decode accounting and padding.
`include/SZ3/decomposition/TimeSeriesDecomposition.hpp`	Validates reconstruction inputs.
`include/SZ3/decomposition/SZBioMDXtcDecomposition.hpp`	Initializes serialized state.
`include/SZ3/decomposition/NoPredictionDecomposition.hpp`	Adds bin validation and sizing.
`include/SZ3/decomposition/InterpolationDecomposition.hpp`	Validates dimensions and metadata.
`include/SZ3/decomposition/Decomposition.hpp`	Adds configuration dependencies.
`include/SZ3/decomposition/BlockwiseDecomposition.hpp`	Validates quantization-bin counts.
`include/SZ3/compressor/SZGenericCompressor.hpp`	Adds RAII and bounded decoding.
`include/SZ3/compressor/specialized/SZTruncateCompressor.hpp`	Adds RAII and updates lossless calls.
`include/SZ3/compressor/specialized/SZExaaltCompressor.hpp`	Updates bounded decoding.
`include/SZ3/api/sz.hpp`	Bounds public decompression.
`include/SZ3/api/impl/SZImplOMP.hpp`	Hardens OpenMP buffers and parsing.
`include/SZ3/api/impl/SZDispatcher.hpp`	Adds RAII and capacity handling.
`include/SZ3/api/impl/SZAlgoInterp.hpp`	Adds memory dependencies.
`include/SZ3/api/impl/SZAlgoBioMD.hpp`	Completes include dependencies.
`CMakeLists.txt`	Bumps the project version.
`.gitignore`	Refines generated-file exclusions.
`.github/workflows/integration_test.yml`	Splits long-running integration jobs.

Review details

Files reviewed: 54/55 changed files
Comments generated: 15
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    std::vector<T> decode(const uchar *&bytes, size_t targetLength, size_t &remaining_length) override {
+        // The reads below are not individually bounded, so check what they consumed before charging it:
+        // subtracting more than is left would wrap remaining_length and unbound everything parsed after.
+        const uchar *decode_start = bytes;


+    std::vector<T> decode(const uchar*& bytes, size_t targetLength, size_t& remaining_length) override {
+        // The reads below are not individually bounded, so check what they consumed before charging it:
+        // subtracting more than is left would wrap remaining_length and unbound everything parsed after.
+        const uchar* decode_start = bytes;


            size_t len = bytesToInt64_bigEndian(bytes) ^ 0x1234abcd;
            bytes += 8;


+        // tree.n is an int holding a 64-bit read, so check the sign before comparing.
+        const size_t dfs_bytes = remaining_length - header_size;
+        if (tree.n < 0 || static_cast<size_t>(tree.n) > dfs_bytes * 8)
+            throw std::out_of_range("SZ3 HuffmanEncoderV2: node count exceeds the compressed buffer");
+        tree.ht.reserve(static_cast<size_t>(tree.n) << 1);


+            if (cnt < 0) {
+                throw std::out_of_range("SZ3 runlength encoder: negative run length");
+            }


+        read(quant_inds_size, bufferPos, bufferSize);
+        auto quant_inds = encoder.decode(bufferPos, quant_inds_size, bufferSize);


+        size_t bufferSize = lossless.decompress(cmpData, cmpSize, buffer, 0);
+        // The parsing below walks the decompressed buffer, so bufferSize is its bound, not cmpSize.
+        size_t remaining_length = bufferSize;


+        for (size_t i = 0; i < 8; i++) {
+            *bytes++ = 0;
+            outSize++;
+        }


+    void load(const unsigned char*& c, size_t& remaining_length) {
+        const unsigned char* const c0 = c;
+        const unsigned char* const cend = c + remaining_length;
        uchar confSize = 0;
-        read(confSize, c);
-        auto c1 = c + confSize;
+        read(confSize, c, remaining_length);


+        const unsigned char* const c1 = std::min(c0 + confSize, cend);

-        read(N, c);
+        read(N, c, remaining_length);


* Stop ALGO_BIOMD writing indeterminate bytes, and charge its load() SZBioMDDecomposition::save() writes firstFillFrame_ and fillValue_ unconditionally, but only compress_2d and compress_3d assign them. A 1D compression therefore put 12 bytes of whatever the object's memory last held into the stream: the same input compressed five times produced five different files. PR #145 gave the same two members an initialiser in SZBioMDXtcDecomposition, which was written from a copy of this file, and left the original. load() then took back everything it had charged. c_pos is the cursor before the reads, so c_pos - c is negative and the last line added the 37 bytes the bounded read() calls had just subtracted. Every later parse -- the encoder's load, the bin count, the decode -- ran on a budget that large again. The Xtc class has no such line. A sweep over the eleven algorithms at one, two and three dimensions puts the determinism defect in ALGO_BIOMD's 1D path alone; of the seventeen places that adjust remaining_length by hand, this is the only one that subtracts a difference taken in the wrong order. test_decomposition_save_load.cpp covers both halves of the contract for both decompositions at all three dimensionalities. Determinism is checked by constructing the decomposition over storage filled with two different patterns and comparing what save() writes, which does not depend on the allocator handing back dirty memory. Reverting either fix fails exactly the cases that fix addresses: the 1D determinism case, and the three accounting cases with "advanced 37 bytes but charged 0". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Run the molecular dynamics algorithms on molecular dynamics data Every SDRBench field the suite covers is 1D or 2D, and every 3D one is a climate or turbulence field, so ALGO_BIOMD and ALGO_BIOMDXTC have never seen the {frames, atoms, xyz} layout they were written for -- which is what GROMACS hands them through the HDF5 filter. fetch_md_trajectory.py downloads the seven MDAnalysisData benchmark trajectories from the figshare files that package points at, checks each against a recorded sha256, and writes the raw arrays the rest of the suite reads. Taking the URLs rather than the package keeps the test data to immutable files; reading DCD, XTC and NetCDF still needs MDAnalysis. The published frame counts for nhaa and yiip are each one short of what the files hold. The script validates the shape it is told against the shape it reads, which is how that surfaced. nhaa and yiip are 911M and 302M elements in full, past what a job can do in half an hour, so those two are cut to their leading 200 and 100 frames. The rest are 2M to 56M. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Cover the BioMD decompositions with unit tests Nothing exercised the fill-frame path. SZBioMDXtcDecomposition skips trailing frames whose values are all equal and writes them back on decompression, and no scientific field has such a frame; a trajectory buffer holding fewer frames than it has room for does. Seven cases over both algorithms: the trajectory layout at four error bounds, trailing fill frames, every frame after the first being fill, a single frame, 1D and 2D input, input shorter than one triplet, and that the same input compresses to the same bytes. Reconstruction rounds to float, so the bound holds to 1.0014x here and to 1.0071x for ALGO_BIOMDXTC through the HDF5 filter, where each chunk quantizes on its own. The cases allow 1.01x. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Record a compression ratio and a timing for every integration case Nothing in the suite looked at either, so a change that halved the ratio or doubled the time passed as long as the error bound held. The per-bit comparison the Huffman decode loop briefly carried cost ALGO_NOPRED 6% of its decompression speed and CI said nothing. Each case now reports its ratio and both timings, the driver collects them into a CSV, and the workflow keeps it as an artifact so two runs can be compared. A ratio below one fails outright: no ratio is guaranteed, but a compressor that grows its input is broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Include what test_biomd uses std::max and std::fill come from <algorithm>, which libstdc++ happens to pull in through one of the other headers and libc++ does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Make the MD tests fail for the reasons they were written for An adversarial read of the suite found four cases that could not report the defect they name. The determinism case ran only at {5, 777, 3}. compressMultiFrame assigns every member save() writes, so the 3D header is a function of the input whatever the memory held; the members that go unassigned are on the 1D and 2D paths. It now runs all three, and fails on master until the ALGO_BIOMD fix. Both fill-frame cases used 1.25f and -3.5f, which sit on the eb = 1e-3 grid, so the quantizer reproduced them exactly whether or not the frames were skipped -- deleting findFillValueAndFirstFilledFrame left the suite green. The values are off-grid now and the tail has to cost less than quantizing it. {1, 1024, 3} is a 2D case: Config::setDims drops a dimension of 1. It keeps its shape, which is what a filter chunking one frame at a time produces, and says so; TwoFrameTrajectory covers the smallest shape that does reach the multi-frame path. kBoundSlack was 1.01, tighter than the eb * 1.1 LinearQuantizer accepts, so a fixture with coordinates in Angstrom rather than nanometres would have failed on a conforming result. Also: the download retried only on an exception, and a connection dropping mid-transfer usually closes cleanly, so a short file failed the one-shot checksum with no retry left. The checksum is inside the loop now and a bad file is removed. The metrics are written from a finally, so a dataset that fails to arrive still leaves the artifact behind. And the HDF5 filter reports a size and a ratio per chunk mode, which the baseline keys by harness -- the path a simulation writes through was the one not being watched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

ayzk force-pushed the master branch from 4ac704f to 57ce9e9 Compare September 1, 2026 17:33

ayzk force-pushed the master-fixes branch 13 times, most recently from 9390f18 to 466d8d5 Compare September 15, 2026 23:59

ayzk and others added 2 commits September 15, 2026 17:13

Include what each header uses

00e42f1

These headers took the standard library names they use from whatever included them first, so they did not compile on their own and broke under a different standard library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ayzk force-pushed the master-fixes branch 5 times, most recently from aa2d3e5 to 097360a Compare September 16, 2026 01:17

ayzk force-pushed the master-fixes branch 5 times, most recently from 95e4ec4 to eb474d8 Compare September 16, 2026 06:14

This was referenced Sep 16, 2026

Fix Config::load reading one byte past the end of the config #131

Closed

Bounds-check decompression against corrupted input #132

Closed

Fix undefined behavior in Huffman encoder for single-symbol input #133

Closed

ayzk force-pushed the master-fixes branch from eb474d8 to 39f72c9 Compare September 16, 2026 06:49

ayzk force-pushed the master-fixes branch from 39f72c9 to bc07ad5 Compare September 16, 2026 06:57

ayzk force-pushed the master-fixes branch from 1423b58 to 651b20e Compare September 16, 2026 17:21

ayzk mentioned this pull request Sep 16, 2026

Exception thrown: LOSSLESS + OpenMP #136

Closed

ayzk mentioned this pull request Sep 16, 2026

Bound the header parse in decompress_config apertus-open-source-cinema/sz3-rs#16

Open

ayzk changed the title ~~[DO NOT MERGE] Pending bug fixes, for review~~ Bounds-check decompression against corrupted input Sep 16, 2026

ayzk marked this pull request as ready for review September 16, 2026 23:20

Copilot AI lite review requested due to automatic review settings September 16, 2026 23:20

ayzk merged commit fbb6f2c into master Sep 16, 2026
20 checks passed

ayzk deleted the master-fixes branch September 16, 2026 23:20

Copilot started reviewing on behalf of ayzk September 16, 2026 23:20 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

This was referenced Sep 17, 2026

Stop ALGO_BIOMD writing indeterminate bytes, and charge its load() #148

Closed

Fix two ALGO_BIOMD defects, and run the MD algorithms on MD data #147

Merged

Navigation Menu

Bounds-check decompression against corrupted input - #145

Bounds-check decompression against corrupted input#145
ayzk merged 6 commits into
masterfrom
master-fixes

ayzk commented Sep 1, 2026 •

edited

Loading

Uh oh!

ayzk commented Sep 16, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		size_t len = bytesToInt64_bigEndian(bytes) ^ 0x1234abcd;
		bytes += 8;

		read(quant_inds_size, bufferPos, bufferSize);
		auto quant_inds = encoder.decode(bufferPos, quant_inds_size, bufferSize);

Conversation

ayzk commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The two module interfaces that change

The pull requests it replaces

Where this departs from the PRs as written

Found while reviewing, fixed here

The third commit — a necessity pass over the second

The fourth commit — what CI and a wider sweep caught

Verification

Uh oh!

ayzk commented Sep 16, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ayzk commented Sep 1, 2026 •

edited

Loading