Skip to content

gzip: initialise state->past for write handles too, not just read handles - #1309

Open
AetherAI3 wants to merge 1 commit into
madler:developfrom
AetherAI3:fix/1303-gz-past-uninit-write-mode
Open

AetherAI3 wants to merge 1 commit into
madler:developfrom
AetherAI3:fix/1303-gz-past-uninit-write-mode

Conversation

@AetherAI3

Copy link
Copy Markdown

Fixes #1303.

gz_reset() initialises state->past only inside the state->mode == GZ_READ branch, but gz_open() allocates gz_state with plain malloc() and does not zero it. Since a4e4521, gzseek64() and gztell64() read state->past in either mode, so a write-mode handle reads past out of indeterminate heap.

In write mode nothing ever writes to past, so the correct value there is always 0 and the fold offset += past ? 0 : skip in gzseek64() should always happen. When the indeterminate byte comes up non-zero the fold is skipped: two SEEK_CUR seeks with no write between them drop the first seek's pending zero-fill, and the resulting gzip stream is well-formed with a valid CRC and the wrong contents. gztell() returns the wrong position after a deferred seek for the same reason, breaking the gztell(file) == gzseek(file, 0L, SEEK_CUR) equivalence documented in zlib.h.

The fix is one line moved: lift state->past = 0 out of the GZ_READ branch so both modes start with it zeroed.

Verified

master at e3dc0a8, -O0 -g -fsanitize=address,undefined build on Debian.

  • A minimal reproducer (gzopen("wb") → two gzseek(5, SEEK_CUR) calls → gzwrite("HELLO", 5) → close → decompress) that expects 15 bytes reproduces the bug five runs out of five under MALLOC_PERTURB_=170 on the unpatched tree (10 bytes back: five zeros dropped) and passes five out of five with the fix in place (15 bytes back: ten zeros + HELLO).
  • make test prints *** zlib test OK *** and *** zlib 64-bit test OK ***.
  • Sanity checks on a normal write with no seek, a single seek + write, and read-past-EOF gzeof() all still behave the same.

Notes

Reported and root-caused by @jxravi.

I used an AI coding assistant while working on this. Every hunk was reviewed by a human before commit, the reproducer and regressions were run on real hardware, and the diff is one line moved with no logic change beyond the reported bug.

Predator CI review 3-model (deepseek-chat CLEAN, glm-4.6 2 issues both FP-verified against gz_open() source, deepseek-r1 CLEAN). No confirmed defects.

…dles

gz_reset() left state->past uninitialised in write mode. gz_open()
allocates gz_state with malloc() rather than calloc(), so a write-mode
handle read the field out of indeterminate heap. Since a4e4521 both
gzseek64() and gztell64() dereference past in either mode, so the
uninitialised byte silently controlled whether two consecutive
SEEK_CUR seeks folded their pending skip into the next offset. When
the byte came up non-zero, the first seek s zero-fill was lost and
the produced gzip stream was well-formed with a valid CRC and the
wrong contents; gztell() likewise returned the wrong position after a
deferred seek.

Move the past = 0 assignment out of the GZ_READ branch so both read
and write handles start with it zeroed. No struct or ABI change.

Fixes: madler#1303
Reported-by: jxravi (github.com/jxravi)
Signed-off-by: Brandon Barrante <aetherai@aethersystems.net>
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.

gzseek()/gztell() read uninitialized state->past in write mode, silently corrupting output (since a4e4521)

1 participant