#53 Insufficient cfactorstr[] size

[Kristýna Streitová]

There was a buffer overflow detected in list.c file where cfactorstr[] had an insufficient size (10). It was fixed in unzip beta versions and expanded to size 12 but I think it's still insufficient. The right size should be 13 (sgn (1), int (10), % (1), nul (1)).

Also, replacing sprintf() by snprintf() might make the code more robust.
Kudos to Josef Möllers (josef.moellers@suse.com).

I'm attaching a patch with changes described above.

Thanks.

[Steven Schweda]

Thanks for the report.

There was a buffer overflow detected in list.c file where cfactorstr[]
had an insufficient size (10). It was fixed in unzip beta versions and
expanded to size 12 but I think it's still insufficient. The right size
should be 13 (sgn (1), int (10), % (1), nul (1)).

The secret is that the result of compr_fract() is (approximately)
divided by ten (for rounding) before it gets used as "cfactor". Look
for expressions like:

            cfactor = (cfactor + 5) / 10;

So, I believe that 1+9+1+1 = 12 is enough.

I hope that no one would write code like this today, and it may be
bad, but I believe that it's better than it looks.

[Kristýna Streitová]

Thanks, I understand. I guess that at least the "snprintf" part of the patch is still relevant and can be used, right?

[Kristýna Streitová]

Just for the tracking purposes: A CVE number was requested for this issue

CVE-2018-18384

[ADITYA RAJ]

Hi Team,

Can we know the expected date of UNZIP 6.1(unzip.exe) release?

Hopefully this Unzip 6.1 Major release will contains fix for this vulnerability “ CVE-2018-18384: Info-ZIP UnZip list.c buffer overflow “ .

Can you please let us know?

Thanks in Advance !

[Steven Schweda]

Can we know the expected date of UNZIP 6.1(unzip.exe) release?

I don't know. There is no firm schedule, and we're looking at some
bugs in the handling of non-ASCII characters on Windows.

Hopefully this Unzip 6.1 Major release will contains fix for this
vulnerability [...]

The next beta kit (6.1c) should have that fix. Eventually, the real
6.1 should have it, too.

[ADITYA RAJ]

Hi Steven,

Is any firm schedule to release unzip 6.1 this year or any expected time has been decided?

Thanks for your support in Advance !!

[Steven Schweda]

Is any firm schedule to release unzip 6.1 this year or any expected
time has been decided?

Nothing's firm or decided. There are some remaining problems with
exotic names on Windows, and this new "zip bomb" thing to deal with. It
should come out this year, but it probably should have come out last
year, too.

[Bill Church]

Seems that Mark Adler has already addressed the zip bomb thing: https://github.com/madler/unzip

Can we please get an official release of Zip/Unzip soon? Right now I'm being told to dump usage of InfoZip because of these vulnerabilities and I'd really rather not as there isn't much in the way of a replacement.

[Moritz Buhl]

This bug is older than this. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=741384

[ADITYA RAJ]

Hi Steven,

Can you please confirm about unzip 6.1 official release timelines? or any planning related to release of unzip 6.1 ?

Thanks

[Steven Schweda]

Can you please confirm about unzip 6.1 official release timelines? or
any planning related to release of unzip 6.1 ?

I haven't done much with it lately. Still no real schedule, but I
have been trying to get some work done on it lately. If I were more
definite, it'd be mostly lies.

[ADITYA RAJ]

Hi Steven/Kristyna,

I have few query related to the patch "unzip_cfactor_overflow.patch" provided for this issue.

1. As I am doing the changes in list.c as provided in the patch. In between I found one method name change i.e sprintf() to snprintf().
   Is the changes of snprintf() is relevant or required to cover this vulnerability?
   or the only change of cfactorstr[] size (13) is sufficient to overcome this issue CVE-2018-18384?

2. After changes in list.c I was also trying to compile the unzip project code through MS Visual studio in order to create unzip.exe
   found complier issue when sprintf() changed to snprintf() i.e
   Error 30 error LNK2019: unresolved external symbol _snprintf referenced in function listfiles list.obj

   But when I changed snprintf() to _snprintf() then the code compiled successfully.
   Can you please elaborate why it is so? Are snprintf() and _snprintf() two different method or Is it single method but name incorrectly in patch?

   Please clarify above points

   Thanks