Skip to content

Harden 7-Zip extraction and LZ41 decompression#408

Open
Goober5000 wants to merge 1 commit intoKnossosNET:mainfrom
Goober5000:fix/7zip_items
Open

Harden 7-Zip extraction and LZ41 decompression#408
Goober5000 wants to merge 1 commit intoKnossosNET:mainfrom
Goober5000:fix/7zip_items

Conversation

@Goober5000
Copy link
Copy Markdown
Contributor

Closes two unfinished items left by recent security commits.

7-Zip path traversal (gap left by e20a3ed):

Commit e20a3ed added per-entry IsSubPath validation to the SharpCompress extraction loops in KnUtils, but the 7-Zip path was unguarded. Both Decompressor.SevenZip mode and Auto mode (which falls back to 7za.exe after a SharpCompress failure) shelled out via SevenZipConsoleWrapper with -aoa -y and let 7-Zip extract any entry, including ../traversal and absolute paths.

  • New SevenZipConsoleWrapper.ListArchiveEntries enumerates entries via "7za l -slt -sccUTF-8" and parses the key=value blocks. Symbolic Link and Hard Link targets are returned alongside Path so the caller can validate them too.
  • DecompressFile now calls a new ValidateArchiveEntries helper before each extraction pass. Any entry that fails IsSubPath aborts extraction with no files written.
  • The .tar.gz two-pass case validates both the outer .tar.gz AND the intermediate .tar between the two Run() calls; if the inner tar fails validation the intermediate is deleted before returning false.
  • StandardOutputEncoding = UTF-8 plus the -sccUTF-8 flag pin the listing output to UTF-8. Without this, on Windows the OEM codepage decoding could mismatch the bytes 7za writes during extraction, allowing a crafted entry to bypass the post-list validation.

LZ41 decompressor hardening (gaps left by 5eb206e):

Commit 5eb206e added floor checks for numOffsets, length, and blockSize plus an endBlock bound, but three vectors remained: the offset-table entries themselves were unchecked (allowed arbitrary inputStream.Position seeks - sibling-file disclosure for VP archives), blockSize had no upper bound (a hostile footer could trigger gigabyte allocations per block), and exceptions in the worker thread were silently swallowed (caller proceeded with uncompressedSize = 0 on a corrupted footer).

  • Each offsets[i] read is now validated against the compressed-data region and required to be strictly greater than the previous entry (matches the compressor's strictly-monotonic emission starting at offset 4).
  • blockSize is capped at 4 MB to match the compressor's Max4MB enum.
  • cmpBytes (offsets[i+1] - offsets[i]) is checked at decode time against LZ4_compressBound(blockSize); offset-table validation alone is insufficient because the gap can legitimately fit within compressedFileSize while exceeding the cmpBuf allocation.
  • VPCompression.DecompressStream now wraps the LZ41 worker thread call in try/catch and rethrows via ExceptionDispatchInfo to preserve the original stack trace. The raw Thread (with explicit 1 MB stack) is kept since LZ4's recursive native decode needs it on platforms where Task.Run can't guarantee the stack size.

Closes two unfinished items left by recent security commits.

7-Zip path traversal (gap left by e20a3ed):

Commit e20a3ed added per-entry IsSubPath validation to the SharpCompress
extraction loops in KnUtils, but the 7-Zip path was unguarded. Both
Decompressor.SevenZip mode and Auto mode (which falls back to 7za.exe
after a SharpCompress failure) shelled out via SevenZipConsoleWrapper
with -aoa -y and let 7-Zip extract any entry, including ../traversal
and absolute paths.

- New SevenZipConsoleWrapper.ListArchiveEntries enumerates entries via
  "7za l -slt -sccUTF-8" and parses the key=value blocks. Symbolic Link
  and Hard Link targets are returned alongside Path so the caller can
  validate them too.
- DecompressFile now calls a new ValidateArchiveEntries helper before
  each extraction pass. Any entry that fails IsSubPath aborts extraction
  with no files written.
- The .tar.gz two-pass case validates both the outer .tar.gz AND the
  intermediate .tar between the two Run() calls; if the inner tar fails
  validation the intermediate is deleted before returning false.
- StandardOutputEncoding = UTF-8 plus the -sccUTF-8 flag pin the listing
  output to UTF-8. Without this, on Windows the OEM codepage decoding
  could mismatch the bytes 7za writes during extraction, allowing a
  crafted entry to bypass the post-list validation.

LZ41 decompressor hardening (gaps left by 5eb206e):

Commit 5eb206e added floor checks for numOffsets, length, and blockSize
plus an endBlock bound, but three vectors remained: the offset-table
entries themselves were unchecked (allowed arbitrary inputStream.Position
seeks - sibling-file disclosure for VP archives), blockSize had no upper
bound (a hostile footer could trigger gigabyte allocations per block),
and exceptions in the worker thread were silently swallowed (caller
proceeded with uncompressedSize = 0 on a corrupted footer).

- Each offsets[i] read is now validated against the compressed-data
  region and required to be strictly greater than the previous entry
  (matches the compressor's strictly-monotonic emission starting at
  offset 4).
- blockSize is capped at 4 MB to match the compressor's Max4MB enum.
- cmpBytes (offsets[i+1] - offsets[i]) is checked at decode time against
  LZ4_compressBound(blockSize); offset-table validation alone is
  insufficient because the gap can legitimately fit within
  compressedFileSize while exceeding the cmpBuf allocation.
- VPCompression.DecompressStream now wraps the LZ41 worker thread call
  in try/catch and rethrows via ExceptionDispatchInfo to preserve the
  original stack trace. The raw Thread (with explicit 1 MB stack) is
  kept since LZ4's recursive native decode needs it on platforms where
  Task.Run can't guarantee the stack size.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant