ruby / ruby/zlib

`Zlib::Deflate#params` segfaults when called before any output buffer exists

Open
#143 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
73
Forks
40
Avg merge
10h 32m
Merged PRs (30d)
2

Description

Zlib::Deflate#params crashes the interpreter with SIGSEGV when it is called on a deflate stream that has not produced output yet. No invalid argument is involved — the same level the stream already has crashes it too.

require 'zlib'
Zlib::Deflate.new(4, 15).params(5, 0)
# => [BUG] Segmentation fault at 0x0000000000000014

Reproduction

The attached zlib-params-segv.rb runs each case in a forked child, so one file reports every result:

1. the crash, and that it does not depend on the argument values
  Deflate.new(4, 15).params(5, 0)                SIGSEGV
  Deflate.new.params(5, 0)                       SIGSEGV
  params with the SAME level it has              SIGSEGV
  raw stream, windowBits -15                     SIGSEGV
  gzip stream, windowBits 31                     SIGSEGV
  after finish, buffer consumed                  SIGSEGV

2. the condition: it crashes exactly when avail_out is 0
  freshly constructed                            avail_out=0
  after writing 100 bytes                        avail_out=1022
  params after writing data                      no crash (exit 0)
  params after avail_out = 4096                  no crash (exit 0)

3. control: constructing without calling params is fine
  Deflate.new(4, 15) and nothing else            no crash (exit 0)
  Zlib.deflate("foo", 4)                         no crash (exit 0)

The after finish case matters: this is not only a "never used the stream yet" state. Any moment where the output buffer has been consumed reaches it again.

Cause

ext/zlib/zlib.c, rb_deflate_params at :1923:

    n = z->stream.avail_out;                                  /* :1934 */
    err = deflateParams(&z->stream, level, strategy);         /* :1935 */
    filled = n - z->stream.avail_out;
    while (err == Z_BUF_ERROR) {                              /* :1937 */
	rb_warning("deflateParams() returned Z_BUF_ERROR");
	zstream_expand_buffer(z);                             /* :1939 */

deflateParams() may emit pending output through deflate(), so it needs next_out/avail_out to be set. zstream_init leaves them at Z_NULL and 0 (:652-653), so on a stream that has not produced output the write goes to a null pointer — the faulting address 0x14 is the offset into that null struct.

The Z_BUF_ERROR loop does call zstream_expand_buffer, but only after the first deflateParams() call, which is the one that crashes.

Every other path guards this. zstream_run at :1163-1164:

    if (z->stream.avail_out == 0) {
	zstream_expand_buffer(z);
    }

Suggested fix

Apply the same guard before the first deflateParams() call:

    level = ARG_LEVEL(v_level);
    strategy = ARG_STRATEGY(v_strategy);

    if (z->stream.avail_out == 0) {
	zstream_expand_buffer(z);
    }

    n = z->stream.avail_out;
    err = deflateParams(&z->stream, level, strategy);

I applied that patch to a build of the released 3.2.3 gem and re-ran the cases above against it, with $LOADED_FEATURES confirming the patched extension was the one loaded. All six crashing cases return normally, the previously working cases are unchanged, and compression still behaves correctly across a mid-stream parameter change: deflating "A" * 2000, calling params(9, Zlib::DEFAULT_STRATEGY), then deflating "B" * 2000 produces 45 bytes that inflate back to the exact input.

Versions

Reproduced on:

  • ruby 3.4.5 with the default gem, Zlib::VERSION 3.2.1, libz 1.3.2
  • the published zlib gem 3.2.3 built from source, same host
  • ruby 4.1.0dev built from ruby/ruby master 973c45fcb3, in-tree Zlib::VERSION 3.2.3

rb_deflate_params is byte-identical between the published 3.2.3 gem and master's in-tree copy, so this is not fixed by the unreleased changes in master. Note that those two 3.2.3 code bases are not otherwise identical: master adds z->stream.state = Z_NULL; to zstream_init, which fixes a separate crash (Zlib.gzip("foo", level: 100)) that the published 3.2.3 still has, and which test/zlib/test_zlib.rb already expects to raise Zlib::StreamError.

poc.txt

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ext/zlib/zlib.c, especially rb_deflate_params and the buffer handling in zstream_run. Run the attached zlib-params-segv.rb reproduction against the extension, then verify the existing zlib tests and the reported mid-stream compression case. Done means all cases exit without SIGSEGV and the compressed data still inflates to the original input.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.