Skip to content

Fix LZXPRESS Huffman decompression - #132

Open
MP-GOWTHAM wants to merge 1 commit into
fox-it:mainfrom
MP-GOWTHAM:fix/lzxpress-huffman-decompression
Open

Fix LZXPRESS Huffman decompression#132
MP-GOWTHAM wants to merge 1 commit into
fox-it:mainfrom
MP-GOWTHAM:fix/lzxpress-huffman-decompression

Conversation

@MP-GOWTHAM

Copy link
Copy Markdown

Fixes the XPRESS LZ77+Huffman (MS-XCA 2.2) decompressor and regenerates its test fixtures, which previously did not match the current decoder semantics.

Decoder fixes

  • Refill only happens at symbol boundaries. The old code preloaded two words and refilled inside skip(), which could consume raw length extension bytes that are interleaved with the bit stream.
  • Handle the raw length extension chain correctly: nibble 15 -> byte, 0xFF -> LE16, LE16 0 -> LE32 (_read_32_bit).
  • Symbol 256 (end-of-data) terminates the stream instead of being misdecoded as a match.
  • Matches are truncated at the 64KiB chunk boundary, per the MS-XCA spec ([MS-XCA] 2.2.1.3).

Fixture regeneration

  • basic and large were re-encoded with a spec-correct chunked encoder; the old fixtures did not decode to their stated digests under the fixed decoder.
  • New le32 fixture exercises the LE32 raw-length extension path and the 64KiB chunk boundary.
  • All fixtures are validated byte-exact by two independent decoders:
    • klauspost/compress Go decoder (AppendHDecompressed, per chunk)
    • Sleuth Kit tsk/fs/xpress.c decoder (TSK vector suite, 41/42 — the single mismatch is a Go single-chunk stream that is incompatible with chunked decoding by design)

Verification

  • pytest: 159 passed, 58 skipped, 0 failed
  • ruff check and ruff format --check: clean

The decoder previously preloaded two words and refilled inside skip(), which could consume raw length extension bytes from the bit stream. Refill now only happens at symbol boundaries, the LE32 raw length extension path is handled, an EOD symbol (256) terminates the stream, and matches are truncated at the 64KiB chunk boundary per MS-XCA.

The basic/large test fixtures were regenerated with a spec-correct chunked encoder (validated byte-exact against the klauspost/compress Go decoder and the Sleuth Kit xpress decoder) and a new le32 fixture exercises the LE32 raw length path and the chunk boundary.
@Schamper

Copy link
Copy Markdown
Member

Can you detail what you used to regenerate the test fixtures?

@MP-GOWTHAM

Copy link
Copy Markdown
Author

The fixtures were regenerated with a small three-stage pipeline (all scripts are scratch, not part of the PR):

  1. Spec-faithful encoder — a ~370-line Python program implementing MS-XCA 2.2 (LZ77+Huffman): it builds the 512-symbol canonical Huffman table from the 256-byte code-length block (even/odd nibbles, per §2.2), emits the bit stream MSB-first as LE16 words, and encodes offset bits per the symbol's hb. It produces three streams:

    • basic (628 B): a typical mixed literal/match stream, with matches capped at length 17 so the stream stays purely bit-packed (symbol codes + offset bits, no raw-length extensions)
    • large (135 KB): a larger mixed corpus spanning multiple 64 KiB chunks
    • le32 (532 B): a hand-crafted two-chunk stream exercising the raw-length extension chain (0xFF -> LE16 0x0000 -> LE32) and the 65536-byte chunk boundary
  2. Independent cross-validation — every stream was decoded by two decoders and compared byte-exact, chunk by chunk:

    • the dissect-util implementation under test (round-trip against the original plaintext)
    • a pure-Go reference decoder (XpressHuffmanDecompress from the klauspost/compress xpress package), which itself was validated against Microsoft's reference vectors in ida/xpress.c
  3. Fixture generation — a small script reads the validated streams and rewrites tests/compression/test_lzxpress_huffman.py: each parameter embeds the compressed stream as hex plus the sha256 digest of the decompressed output as the oracle (keeps the test compact, and both decoders had to agree on every digest before it was written).

Happy to commit the encoder + generator into the repo (e.g. under tests/compression/ or a tools dir) if you'd like the fixtures to be reproducibly regenerated.

@StrongWind1

Copy link
Copy Markdown

Does this overlap with my PR? #129

"Fixes two bugs in lzxpress_huffman decompress."

@MP-GOWTHAM

Copy link
Copy Markdown
Author

Good question - let me give you a concrete answer instead of a hand-wave, since I already have both decoders locally to compare.

What overlaps (both PRs touch the same function):

  1. Symbol 256 (end-of-data) terminates the stream instead of being decoded as a match.
  2. The raw-length escape chain now includes the v10.0 uint16(0) -> uint32 path.

What does NOT overlap - and I think this is the important part. I ran my regenerated fixtures against each decoder:

decoder basic large le32 (64 KiB boundary + LE32 escape)
current main fail fail fail
your #129 decoder pass pass fail
your #129 + match truncation pass pass pass
my #132 decoder pass pass pass

The le32 fixture fails under #129's decoder (wrong output, no exception). It is the fixture that exercises a match crossing the 64 KiB chunk boundary. MS-XCA 2.2.1.3 requires truncating the copy at the chunk boundary; #129 keeps the unbounded copy loop, so the chunk overshoots and everything after it is misparsed. Adding the one-line truncation (length = min(length, 65536 - chunk_size)) to your decoder makes all three fixtures pass, which pins down the missing fix.

My PR additionally refactors refilling so it only happens at symbol boundaries: raw length bytes are read before the offset bits are consumed, instead of peeking the offset first and refilling inside skip(), which can pull words from past the interleaved raw bytes.

And the fixtures genuinely needed regenerating - the current main-branch decoder fails all three of mine, so the old digests were only consistent with the old decoder.

To avoid duplication, I suggest merging #129 first (it is the larger feature PR), then I rebase #132 on top and keep only the delta: the truncation fix, the refill refactor, and the regenerated fixtures. Alternatively, if you would rather have a single PR, I can close #132 and port the delta onto #129 directly. Happy to go either way - let me know your preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants