Skip to content

Reset next_in and avoid undefined behavior in gzwrite.c - #1320

Open
filtede98 wants to merge 1 commit into
madler:developfrom
filtede98:fix-gzwrite-dangling-next-in
Open

filtede98 wants to merge 1 commit into
madler:developfrom
filtede98:fix-gzwrite-dangling-next-in

Conversation

@filtede98

Copy link
Copy Markdown

In gzwrite.c, several edge cases and undefined behavior issues exist when mixing direct user-buffer writes (len >= state->size) with subsequent gzprintf() / gzvprintf() calls:

1. Dangling next_in pointer after successful large writes in gz_write

When gz_write compresses directly from a user buffer (len >= state->size), it sets state->strm.next_in = (z_const Bytef *)buf;. In commit df84af2 (remediation for CVE-2026-85091), state->strm.next_in was correctly reset to state->in upon error (ret == -1). However, on the successful loop exit (len == 0), state->strm.next_in is never reset to state->in.
If the caller allocated buf on the stack or subsequently frees buf, state->strm.next_in is left as a dangling pointer into invalidated/unmapped memory.

2. Undefined behavior pointer comparison in gz_vacate

When gzprintf() or gzvprintf() is called, it invokes gz_vacate(state) to ensure buffer space for formatted output. gz_vacate checks:

if (strm->next_in == NULL ||
    strm->next_in + strm->avail_in <= state->in + state->size)
    return 0;

If a preceding direct write left strm->next_in pointing to user memory:

  • Comparing pointers that do not belong to the same object or array (strm->next_in vs state->in) using <= is Undefined Behavior under ISO C (C99/C11/C17 §6.5.8p5).
  • If the user buffer resides at an address numerically greater than state->in + state->size, the condition evaluates to false even though the input buffer is empty (strm->avail_in == 0). This causes gz_vacate to needlessly run gz_comp(state, Z_NO_FLUSH) on an empty buffer.
    When strm->avail_in == 0, the input buffer is already empty and there is nothing to vacate. Guarding strm->avail_in == 0 at entry to immediately restore strm->next_in = state->in and return 0 eliminates the disparate pointer comparison and avoids spurious compression calls.

3. Redundant pointer subtraction in gzvprintf

In gzvprintf (line 457):

next = (char *)(state->in + (strm->next_in - state->in) + strm->avail_in);

Subtracting state->in from strm->next_in across different memory objects is also undefined behavior. In gzprintf (line 574), this was already simplified to (strm->next_in + strm->avail_in). Aligning gzvprintf with gzprintf removes this redundant subtraction.

4. Test coverage

Added an automated test in test/example.c (test_gzio) verifying that a large buffer gzwrite followed immediately by gzprintf executes cleanly without state corruption or sanitizer warnings.

@madler

madler commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Please test the original issue against the current develop branch. There have been several related commits in the last day. Thanks.

@filtede98

Copy link
Copy Markdown
Author

Hi Mark (@madler),

Thanks for checking in! I have tested and verified this against the latest develop branch (commit 767c4c9). The issue remains fully reproducible and active on current develop.

Here is the exact trace of why it persists:

  1. Dangling next_in in gz_write (gzwrite.c:234-254):
    When compressing directly from a user buffer (len >= state->size), state->strm.next_in is set to buf. On error (ret == -1), state->strm.next_in is correctly restored to state->in at line 247. However, on the successful loop termination (len == 0 at line 250), state->strm.next_in is never reset to state->in. If the caller's buffer was allocated on the stack or subsequently freed, state->strm.next_in remains pointing to invalidated memory.

  2. Undefined Behavior comparison in gz_vacate (gzwrite.c:389-391):
    When a subsequent gzprintf() or gzvprintf() call invokes gz_vacate(), it checks:

    if (strm->next_in == NULL ||
        strm->next_in + strm->avail_in <= state->in + state->size)
        return 0;

    When strm->next_in points to the prior user buffer rather than state->in, comparing two disparate pointers with <= is undefined behavior (ISO C99/C11 §6.5.8p5). Furthermore, if buf happens to be located at an address numerically above state->in + state->size, the condition evaluates to false even though strm->avail_in == 0, triggering an unnecessary gz_comp(state, Z_NO_FLUSH) on an empty buffer.

  3. Disparate pointer subtraction in gzvprintf (gzwrite.c:457):

    next = (char *)(state->in + (strm->next_in - state->in) + strm->avail_in);

    Subtracting state->in from strm->next_in across different allocated objects is also undefined behavior (§6.5.6p9). Note that gzprintf (line 574) was already simplified to (strm->next_in + strm->avail_in).

None of the recent commits to develop (6995c67, 6ed77c4, 930cdf0, ec63cd6, 767c4c9) touched gzwrite.c (they focused on gzseek.c, gzread.c, gzguts.h, and deflate.c).

The PR is rebased cleanly on top of HEAD of develop and ensures:

  • state->strm.next_in is unconditionally reset to state->in upon completing direct writes.
  • gz_vacate early-returns when strm->avail_in == 0 without performing cross-object pointer arithmetic.
  • gzvprintf matches gzprintf's direct pointer arithmetic.

Thank you!

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.

2 participants