indygreg / indygreg/python-zstandard

Segfault: 4+ unchecked `PyBytes_AsStringAndSize` on user `read()` return uses uninitialized memory

Open
#292 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
642
Forks
116
Avg merge
1d 14h
Merged PRs (30d)
5

Description

## Summary

Several streaming paths call `PyBytes_AsStringAndSize(result, &readBuffer, &readSize)` on the return value of a caller-supplied `source.read()` and do not check the return value. When `read()` returns a non-`bytes` object (e.g., `str`, `None`, `bytearray`), `readBuffer` and `readSize` are left with their prior/uninitialized contents; the next `ZSTD_compressStream2` call reads from those addresses. Observed: SEGV on release builds, `_Py_CheckFunctionResult` abort on debug builds.

## Impact

- **Severity:** SEGV on release builds; assertion abort on debug builds.
- **Reachability:** Any caller-supplied `source.read()` whose return is not `bytes`. A trivial wrapper around a text file or a mistakenly-returned `bytearray` triggers it.
- **Version:** 0.25.0 (commit `7a77a75`).
- **Platform:** Confirmed Linux x86_64 / CPython 3.14 debug; bug is platform-independent.

## Reproducers

**SEGV on release — via `copy_stream`:**

```python
import zstandard, io

class BadSource:
def read(self, size):
return 'not bytes' # str, not bytes

comp = zstandard.ZstdCompressor()
comp.copy_stream(BadSource(), io.BytesIO())
# Segmentation fault
```

**Assertion abort on debug — via iterator:**

```python
import zstandard

class BadSource:
def read(self, size):
return 'not bytes'

comp = zstandard.ZstdCompressor()
it = comp.read_to_iter(BadSource())
next(it)
# Fatal Python error: _Py_CheckFunctionResult: a function returned a result with an exception set
# TypeError: expected bytes, str found
```

## Root cause

`PyBytes_AsStringAndSize` returns `-1` and sets a `TypeError` when its argument is not a `bytes` object. On failure, the by-address output parameters `readBuffer` / `readSize` are not written. zstandard ignores the return code and proceeds to use those addresses, passing them to `ZSTD_compressStream2` which reads from whatever happens to be on the stack / in registers.

## Affected sites

| File | Line | Function |
|------|------|----------|
| `c-ext/compressor.c` | 349 | `copy_stream` |
| `c-ext/decompressor.c` | 202 | `decompressor_copy_stream` |
| `c-ext/compressoriterator.c` | 98 | `ZstdCompressorIterator_iternext` |
| `c-ext/decompressoriterator.c` | 134 | `ZstdDecompressorIterator_iternext` |

Plus 2 additional sites reported in the full analysis (in `read_compressor_input` and the decompressor equivalent).

## Suggested fix

Mechanical — add the standard error check after every call:

```c
if (PyBytes_AsStringAndSize(result, &readBuffer, &readSize) < 0) {
Py_DECREF(result);
goto finally;
}
```

Optionally: tighten the documented API contract on `source.read()` to specify that the return must be a `bytes` object (the C code already expects this). Enforcing it at the boundary would be a small additional cleanup.

## Methodology

Found via [cext-review-toolkit](https://github.com/devdanzin/cext-review-toolkit) (Tree-sitter-based static analysis with structured naive/informed review passes). SEGV on release verified at the `copy_stream` site; assertion abort on debug verified at the iterator site. Four sites confirmed via direct reproducer; two more confirmed via static review. Happy to open a PR — the fix is a ~8-line diff.

*Discovery, root-cause analysis, and issue drafting were performed by [Claude Code](https://www.anthropic.com/claude-code) and reviewed by a human before filing.*

## Full report

Complete multi-agent analysis (48 FIX findings across 13 categories, plus a reproducer appendix): https://gist.github.com/devdanzin/b86039ac097141579590c1a0f3a43605

Contributor guide

Open the contributing guide

Research direction

Start in c-ext/compressor.c, c-ext/decompressor.c, c-ext/compressoriterator.c, and c-ext/decompressoriterator.c at the listed PyBytes_AsStringAndSize calls, then inspect the additional calls in read_compressor_input and its decompressor equivalent. Reproduce with the supplied BadSource examples and verify that non-bytes returns raise TypeError without a crash or assertion abort at every affected streaming path.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.