bug#63848: closed ([PATCH] Incorrect usage of inflate() from zlib in decompress.c)

[GNU bug Tracking System]

Your message dated Thu, 08 Jun 2023 12:43:11 +0300
with message-id <83cz262ssw.fsf@gnu.org>
and subject line Re: bug#63832: fix failed inflation of .el.gz archives due to 
passing empty buffer to inflate()
has caused the debbugs.gnu.org bug report #63832,
regarding [PATCH] Incorrect usage of inflate() from zlib in decompress.c
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
63832: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63832
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems

--- Begin Message --- Subject: [PATCH] Incorrect usage of inflate() from zlib in decompress.c Date: Fri, 2 Jun 2023 16:44:25 +0530

Hey there,

 

While building Emacs one of my friends encountered a bug where entering certain commands such as `M-x eww RET`
After some digging in, we found this was because of Z_BUF_ERROR arising in decompress.c:150

Per the docs (inflate (linuxbase.org)) this happens when either of in or out buffer runs out and there is no further possible progress

The code makes a wrong assumption that IF `stream.avail_out` is zero, we can keep on inflating. It’s possible for `stream.avail_in` and `stream.avail_out` to be both zero at the same time (I don’t have a minimalistic test case for this yet, but I am sure that one can construct this with some thought)

Following is the patch for the fix

--
--- a/src/decompress.c

+++ b/src/decompress.c

@@ -151,7 +151,7 @@ md5_gz_stream (FILE *source, void *resblock)

        return -1;

 

       accumulate_and_process_md5 (out, MD5_BLOCKSIZE - stream.avail_out, &ctx);

-    } while (stream.avail_in && !stream.avail_out);

+    } while (!stream.avail_out);

 

   } while (res != Z_STREAM_END);
--

This is also my first time contributing so I might have made mistakes in making a good patch (one is obviously my incapability to quickly think of minimal test case), so I will appreciate suggestions

 

Thanks,

Deepak Sharma

--- End Message ---

> --- Begin Message ---
>
>
>
> Subject: 
>
> Re: bug#63832: fix failed inflation of .el.gz archives due to passing empty buffer to inflate()
>
>
>
>
> Date: 
>
> Thu, 08 Jun 2023 12:43:11 +0300
>
>
>
>
> > From: Amritpal Singh <icy.amrit@gmail.com>
> > Date: Fri, 2 Jun 2023 12:21:07 +0530
> > 
> > Compile emacs with the system's gzip program set to `pigz`.
> > Run emacs and then `M-x eww RET`
> > 
> > Expected behavior:
> > Enter URL prompt in mini-buffer
> > 
> > Actual behavior:
> > hashing failed '/usr/share/emacs/30.0.50/lisp/gnus/gnus.el.gz'
> > 
> > Report:
> > The bug has been reproduced on emacs version 29.0.91 and HEAD which
> > seems to be at 30.0.50.
> > Later, a copy of the aforementioned file was saved somewhere else and
> > the program was uninstalled. Then emacs was recompiled with system's
> > gzip program set to GNU gzip and the initial steps were repeated and
> > the expected behavior was the result.
> > This lead to believing either that there's a bug with how zlib's
> > `inflate()` handles archives or emacs code was having an issue with
> > archives files.
> > 
> > The hashes for gz archives generated with different programs were as follows
> > > md5sum gnus-gzip.el.gz
> > edb3d0ffba7f19ff1d4ec3f889609e8a  gnus-gzip.el.gz
> > > md5sum gnus.el.gz
> > 985deaaec6a5845ac8d6bd9648957b50  gnus.el.gz
> > 
> > And when uncompressing these archives, the resulting file was the same
> > and the hash for the files was the same (omitted for brevity).
> > 
> > Now after logging some code in $EMACS_REPO/src/decompress.c, it was
> > learned that in the pigz specific case, `inflate()` was returning
> > Z_BUF_ERROR(-5) which is an indicator for zstream's either `avail_in`
> > or `avail_out` fields are 0.
> > 
> > Observing the code in `$EMACS_REPO/src/decompress.c`
> > L154:
> >     } while (!stream.avail_out);
> > only checks stream.avail_out and not stream.avail_in which also might
> > have been set to 0. A special case here can be constructed where
> > `avail_in` is 0, and the code keeps looping even though our input
> > buffer is empty and thus causing a Z_BUF_ERROR. Placing a simple check
> > for it fixes the bug in pigz's gz archives case and does not cause any
> > issue with gzip archives.
> > 
> > A patch with a simple fix is attached below
>
> Thanks, installed on the master branch, and closing the bug.
>
>
> --- End Message ---