Hello,
I have found 2 bugs in compress(1) using AFL++ fuzzing.
The patches below are just examples, feel free to solve another way.
1) fd double-close on invalid input
When gz_ropen() or zip_ropen() fail to parse the input header,
they call gz_close()/zip_close() which closes the fd. The caller
dodecompress() then closes the same fd again.
Call path for the zip case:
dodecompress() opens file, gets ifd
check_method(ifd) reads magic bytes
zip_ropen(ifd, ...) stores fd in s->z_fd
get_header() fails on malformed input
zip_close() closes s->z_fd (== ifd), frees s
returns NULL
close(ifd) double-close
Same pattern exists in gz_ropen() via gz_close(). z_ropen() is
not affected because it does not close the fd on failure.
Fix: on header parse failure, just call inflateEnd() and free the
allocated state instead of calling the full close function. This
leaves fd ownership with the caller.
2) uint16_t underflow in zip extra field parsing
In get_header(), extlen is uint16_t. The EF_TIME, EF_IZUNIX, and
EF_ZIP64 handlers subtract from extlen without bounds checking.
A crafted zip with a small extlen but EF_TIME or EF_IZUNIX extra
fields causes extlen to wrap around to ~65K, making the parser
read far past the extra field area.
Example with extlen=5 and an EF_TIME field with the mtime flag:
extlen = 5
read sig + fieldlen: extlen -= 4 -> extlen = 1
EF_TIME: read flags: extlen-- -> extlen = 0
mtime flag set: extlen -= 4 -> extlen = 65532 (wrap)
while (extlen >= 4) continues for ~16K more iterations
Fix: reject fields where fieldlen exceeds extlen, and check
fieldlen before reading optional sub-fields in EF_TIME and
EF_IZUNIX handlers.
Index: usr.bin/compress/gzopen.c
===================================================================
RCS file: /cvs/src/usr.bin/compress/gzopen.c,v
retrieving revision 1.35
diff -u -p -r1.35 gzopen.c
--- usr.bin/compress/gzopen.c 18 Jun 2022 03:23:19 -0000 1.35
+++ usr.bin/compress/gzopen.c
@@ -138,8 +138,9 @@ gz_ropen(int fd, char *name, int gotmagi
/* read the .gz header */
if (get_header(s, name, gotmagic) != 0) {
- gz_close(s, NULL, NULL, NULL);
- s = NULL;
+ (void)inflateEnd(&s->z_stream);
+ free(s);
+ return NULL;
}
return s;
Index: usr.bin/compress/zipopen.c
===================================================================
RCS file: /cvs/src/usr.bin/compress/zipopen.c,v
retrieving revision 1.1
diff -u -p -r1.1 zipopen.c
--- usr.bin/compress/zipopen.c 22 Oct 2022 14:41:27 -0000 1.1
+++ usr.bin/compress/zipopen.c
@@ -193,6 +193,10 @@ get_header(struct zip_state *s, char *na
s->z_hlen += 4;
extlen -= 4;
+ /* Field extends past the extra field area. */
+ if ((uint16_t)fieldlen > extlen)
+ break;
+
switch (sig) {
case EF_ZIP64:
/* 64-bit file sizes */
@@ -206,12 +210,14 @@ get_header(struct zip_state *s, char *na
break;
case EF_TIME:
/* UTC timestamps */
+ if (fieldlen < 1)
+ break;
if ((c = get_byte(s)) == EOF)
break;
s->z_hlen++;
extlen--;
fieldlen--;
- if (c & 1) {
+ if ((c & 1) && fieldlen >= 4) {
got_mtime = 1;
s->z_time = get_uint32(s);
s->z_hlen += 4;
@@ -223,6 +229,8 @@ get_header(struct zip_state *s, char *na
/* We prefer EF_TIME if it is present. */
if (got_mtime)
break;
+ if (fieldlen < 8)
+ break;
/* skip atime, store mtime. */
(void)get_uint32(s);
@@ -276,8 +284,9 @@ zip_ropen(int fd, char *name, int gotmag
/* Read the zip header. */
if (get_header(s, name, gotmagic) != 0) {
- zip_close(s, NULL, NULL, NULL);
- s = NULL;
+ (void)inflateEnd(&s->z_stream);
+ free(s);
+ return NULL;
}
return s;
