Bug 2164068

Created attachment 1940228 [details]
archive.zip harvested from the failing Python test

Reproduce the buffer overflow with:

<mock-chroot> sh-5.2# rpm -q unzip
unzip-6.0-59.fc38.x86_64

<mock-chroot> sh-5.2# unzip -t archive.zip 
Archive:  archive.zip
*** buffer overflow detected ***: terminated

OK, this fails with arbitrary zips as well:

<mock-chroot> sh-5.2# touch a
<mock-chroot> sh-5.2# zip test.zip a
  adding: a (stored 0%)
<mock-chroot> sh-5.2# unzip -t test.zip 
Archive:  test.zip
*** buffer overflow detected ***: terminated

I've rebuilt unzip in mock and I was still able to reproduce the problem.

I've added `%global _fortify_level 2` and rebuilt it again. The problem was gone:

<mock-chroot> sh-5.2# touch a
<mock-chroot> sh-5.2# zip test.zip a
  adding: a (stored 0%)
<mock-chroot> sh-5.2# unzip -t test.zip 
Archive:  test.zip
    testing: a                        OK
No errors detected in compressed data of test.zip.


Hence, blocking bz2158232 here and will open  PR shortly to set the fortify level to 2 until this is solved.

https://src.fedoraproject.org/rpms/unzip/pull-request/6

The code in unzip:

2884            /* Convert filtered wide chars back to multi-byte. */
2885            woslen = wcstombs( NULL, wostring, 0);
2886            if ((newraw = malloc(woslen + 1)) == NULL) {
2887                free(wstring);
2888                free(wostring);
2889                strcpy( (char *)space, raw);
2890                return (char *)space;
2891            }
2892            woslen = wcstombs( newraw, wostring, (woslen * MB_CUR_MAX) + 1);

Siddhesh says that this is an unzip bug as it passes the wrong buffer size. I tend to agree, although overflow is not possible in this case (because unzip is single-threaded etc.).

So this looks like a thinko; one really only needs woslen here, not woslen * MB_CUR_MAX.  I've sent a PR:

https://src.fedoraproject.org/rpms/unzip/pull-request/7

For additional context, the fortified wcstombs (which predates _FORTIFY_SOURCE=3, the new level just happens to succeed more often) expects the passed length to be within bounds of the size of newraw, which makes sense for the function to be safe because any larger size essentially is a pass to read beyond bounds.  An actual buffer overflow check would be quite expensive and is avoided because there's no non-buggy reason to do this.

Please test:

https://bodhi.fedoraproject.org/updates/FEDORA-2023-cee3cccdb9

Build in progress in https://koschei.fedoraproject.org/package/python3.12?collection=f38

Manual check:

$ koji wait-repo f38-build --build unzip-6.0-60.fc38
Successfully waited 0:00 for unzip-6.0-60.fc38 to appear in the f38-build repo

$ mock -r fedora-rawhide-x86_64 init
...
$ mock -r fedora-rawhide-x86_64 --enablerepo=local --update unzip
...
Upgraded:
  unzip-6.0-60.fc38.x86_64
...
$ mock -r fedora-rawhide-x86_64 shell
...
<mock-chroot> sh-5.2# touch a
<mock-chroot> sh-5.2# zip test.zip a
  adding: a (stored 0%)
<mock-chroot> sh-5.2# unzip -t test.zip
Archive:  test.zip
    testing: a                        OK
No errors detected in compressed data of test.zip.

Same issue exists for zip package, the reported error is the same, even the fix (fortify_source  2) works for it.

I was able to reproduce it with:

zip a52.zip -r Inbox/
*** buffer overflow detected ***: terminated


zip error: Interrupted (aborting)

(In reply to Lucian Langa from comment #9)
> Same issue exists for zip package, the reported error is the same, even the
> fix (fortify_source  2) works for it.
> 
> I was able to reproduce it with:
> 
> zip a52.zip -r Inbox/
> *** buffer overflow detected ***: terminated
> 
> 
> zip error: Interrupted (aborting)

It's likely a different bug because zip does not have any calls to wcstombs and doesn't appear to share that code with unzip.  I'll try to reproduce it.

Not reproducible with a simple local test on rawhide:

# ls somedir
NETBOOT_METHOD.TXT  RECIPE.TXT
# zip test.zip -r somedir/
updating: somedir/ (stored 0%)
  adding: somedir/NETBOOT_METHOD.TXT (stored 0%)
  adding: somedir/RECIPE.TXT (stored 0%)

I even tried compressing a glibc source tree and the zip30 source tree, but no crash.

Can you please file a separate bug with a minimal reproducer for zip?

This bug appears to have been reported against 'rawhide' during the Fedora Linux 38 development cycle.
Changing version to 38.

This does indeed appear to be fixed in 6.0-60