Closed asarubbo closed 9 months ago
I found two stack buffer overflow (maybe the same root cause).
It's two instances of the same problem. One results in "printf of non-null-terminated string", and the other is an out-of-bounds read while copying the filename.
For -l
, it's crashing in this printf:
filename_inzip
is declared here:
and gets filled by
The problem is that unzGetCurrentFileInfo64
will only null terminate the filename if there is room left in the buffer:
If there's not enough room, the buffer is filled to its capacity, without any null terminator.
The -x
crash happens for similar reasons - filename_inzip
not being null terminated - here:
The easiest way to fix this is probably to have the calling code always null-terminate the filename_inzip
buffer, by doing something like:
char filename_inzip[257] = {0};
...
err = unzGetCurrentFileInfo64(uf,&file_info,filename_inzip,sizeof(filename_inzip) - 1,NULL,0,NULL,0);
That would fix the crashes in miniunz, but it's pretty likely that there's other code using minizip which has the same problem. (See e.g. this old Chromium fix.) It would perhaps be worth considering a deeper fix, making the API less dangerous by ensuring that unzGetCurrentFileInfo64
always null terminates the buffer, though that could break any code which relies on the old behavior.
@zmodem: A fix has been done for it?
cc: @madler
Can you look with the latest release build, 1.3.1?
Can you look with the latest release build, 1.3.1?
The problem is still there:
$ clang -fsanitize=address -g contrib/minizip/miniunz.c contrib/minizip/unzip.c contrib/minizip/ioapi.c inflate.c crc32.c adler32.c inftrees.c inffast.c zutil.c && ./a.out -l 2.crashes.zip
...
==3861855==ERROR: AddressSanitizer: stack-buffer-overflow on address
Making the API safer would be good, but could also be more disruptive, so I sent https://github.com/madler/zlib/pull/911 to just fix the calling code in miniunz.c.
Fixed by increasing buffer sizes.
Fixed by increasing buffer sizes.
By looking at the git history I don't see any commits that mentioned this. Are you referring to 15c45adb76e81a7e3a8a9e17b2a56eb90f668f44 or what is the commit the fixes this issue?
@asarubbo: @madler has not pushed computer changes, I have already specified in another ticket but he has not pushed yet :/
@madler: Can you push all your changes after 1.3.1 release build?
Thanks in advance.
Hello,
by fuzzing miniunz in zlib-1.3 I found two stack buffer overflow (maybe the same root cause). They are reproducible with the same testcase by just switching from list (-l) to extract (-x).
The first:
The second:
If I can do further, please let me know. Testcase: 2.crashes.zip