Skip to content

gh-156057: Wrap compression library exceptions in tarfile.ReadError - #156143

Open
JMak-Security wants to merge 3 commits into
python:mainfrom
JMak-Security:gh-156057-tarfile-wrap-compression-errors
Open

gh-156057: Wrap compression library exceptions in tarfile.ReadError#156143
JMak-Security wants to merge 3 commits into
python:mainfrom
JMak-Security:gh-156057-tarfile-wrap-compression-errors

Conversation

@JMak-Security

Copy link
Copy Markdown

Fixes gh-156057.

The bug

TarFile.extractfile() reads a member's payload through _FileInFile.read(), which calls the underlying compressed fileobj.read() (a gzip.GzipFile/bz2.BZ2File/lzma.LZMAFile/zstd.ZstdFile for seekable-mode r:gz/r:bz2/r:xz/r:zst archives) with no exception handling. If the member's compressed data is corrupted after the header was already read successfully (rather than at initial archive-open time), the codec's own exception - zlib.error, OSError from bz2, lzma.LZMAError, or zstd.ZstdError - leaks straight through to the caller instead of being wrapped in tarfile.ReadError.

The streaming-mode (r|gz etc.) code path already handles this correctly: _Stream sets a self.exception attribute to the specific codec's error type and catches it around its own decompress() calls. The seekable-mode path via _FileInFile had no equivalent.

The fix

Give TarFile (and _FileInFile) the same per-codec exception attribute pattern already used by _Stream:

  • TarFile.exception defaults to OSError (correct for bz2, and for uncompressed tar mode where it's simply unused).
  • gzopen()/xzopen()/zstopen() set it to the specific type (zlib.error, LZMAError, ZstdError) after a successful open, mirroring what they already do for t._extfileobj.
  • ExFileObject.__init__ passes tarfile.exception through to _FileInFile, which now wraps its fileobj.read() call and re-raises as tarfile.ReadError.

Verification

Reproduced the leak against current main for gzip, bzip2, and xz (couldn't test zstd locally - no compression.zstd module in the Python available here) using a corrupted-mid-stream archive for each, confirming all three previously leaked the raw codec exception. Same test against the patched version confirms all three now raise tarfile.ReadError instead. Also ran a full write/read/extract round-trip across all four compression modes (uncompressed, gzip, bzip2, xz) to confirm no regression in normal operation.

…rror

_FileInFile.read() (used by TarFile.extractfile() for seekable-mode
archives) called the underlying compressed fileobj's read() directly.
If a member's compressed payload was corrupted after the header had
already been read successfully, the codec's own exception (zlib.error,
OSError from bz2, lzma.LZMAError, or zstd.ZstdError) would leak straight
through instead of being wrapped in tarfile.ReadError, unlike the
existing streaming-mode (r|gz etc.) code path in _Stream, which already
handles this correctly via a self.exception attribute set per codec.

Give TarFile (and _FileInFile) the same per-codec exception attribute,
set by gzopen/xzopen/zstopen to the specific error type (bz2 already
matches the OSError class default), and wrap the read() call in
_FileInFile with it.

Verified by reproducing the leak against current main for gzip, bzip2,
and xz (zstd untestable here, no compression.zstd module available in
the local Python used to verify this), confirming the same input now
raises tarfile.ReadError for all three, and running a full
read/write/extract round-trip across all four compression modes to
confirm no regression.
The check-warnings CI job correctly flagged :meth:TarFile.extractfile`n as unresolvable - Sphinx needs the fully qualified module.Class.method
path for cross-file references. Use tarfile.TarFile.extractfile instead,
matching how Doc/library/tarfile.rst defines it under the tarfile module.

@Aniketsy Aniketsy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JMak-Security thanks for the PR

Could you add tests for this ?

@JMak-Security

JMak-Security commented Aug 21, 2026

Copy link
Copy Markdown
Author

@Aniketsy, sure, I'll get back to you later when I add the tests.

Adds direct unit tests for _FileInFile.read()'s new exception-wrapping
behavior (both the positive case, wrapping the configured exception
type into ReadError, and the negative case, letting an unconfigured
exception type propagate unwrapped), plus an integration test
confirming the real gzopen() -> ExFileObject -> _FileInFile wiring
surfaces a zlib.error raised mid-extraction as tarfile.ReadError.

Verified these tests fail against the pre-fix source (TypeError on the
new exception= parameter for the unit tests; the raw zlib.error
propagating uncaught for the integration test) and pass against the
post-fix source.

Signed-off-by: Jason Mak <squrrielbro@gmail.com>
@JMak-Security

Copy link
Copy Markdown
Author

@Aniketsy done! Added Lib/test/test_tarfile.py::FileInFileExceptionWrappingTest (direct unit tests on _FileInFile.read(): one confirming it wraps the configured exception type into ReadError, one confirming an unconfigured exception type still propagates unwrapped) and GzipExtractfileWrapsCompressionErrorTest (an integration test confirming the real gzopen() wiring surfaces a zlib.error raised mid-extraction as tarfile.ReadError).

I verified these against both states of Lib/tarfile.py: they fail on the pre-fix source (TypeError: unexpected keyword argument 'exception' for the unit tests, and the raw uncaught zlib.error for the integration test) and pass on the post-fix source, so they should meaningfully guard this behavior going forward.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tarfile: member reads can propagate underlying compression exceptions instead of wrapping them

2 participants