Bug#1054290: zlib: CVE-2023-45853

[Salvatore Bonaccorso]

Source: zlib
Version: 1:1.2.13.dfsg-3
Severity: important
Tags: security upstream
X-Debbugs-Cc: car...@debian.org, Debian Security Team <te...@security.debian.org>

Hi,

The following vulnerability was published for zlib.

CVE-2023-45853[0]:
| MiniZip in zlib through 1.3 has an integer overflow and resultant
| heap-based buffer overflow in zipOpenNewFileInZip4_64 via a long
| filename, comment, or extra field. NOTE: MiniZip is not a supported
| part of the zlib product.


If you fix the vulnerability please also make sure to include the
CVE (Common Vulnerabilities & Exposures) id in your changelog entry.

For further information see:

[0] https://security-tracker.debian.org/tracker/CVE-2023-45853
https://www.cve.org/CVERecord?id=CVE-2023-45853
[1] https://github.com/madler/zlib/pull/843
[2] https://github.com/madler/zlib/commit/73331a6a0481067628f065ffe87bb1d8f787d10c

Please adjust the affected versions in the BTS as needed.

Regards,
Salvatore

[James Addison]

Source: zlib
Followup-For: Bug #1054290
X-Debbugs-Cc: car...@debian.org

Although this bug exists in src:zlib, the only binary package affected is, I
believe, the 'minizip'[1] package.

A fix[2] for CVE-2023-45853 has been applied upstream, and is pending[3] an
upstream release.

Please find attached a patch to apply the commit containing the fix to the
Debian source package.

[1] - https://tracker.debian.org/pkg/minizip

[2] - https://github.com/madler/zlib/commit/73331a6a0481067628f065ffe87bb1d8f787d10c

[3] - https://github.com/madler/zlib/issues/868

[James Addison]

Source: zlib
Followup-For: Bug #1054290

X-Debbugs-Cc: car...@debian.org, Debian Security Team <te...@security.debian.org>

I wrote:
> Although this bug exists in src:zlib, the only binary package affected is, I
> believe, the 'minizip'[1] package.

This turns out to be a half-truth: the affected minizip code is vendored into
other source packages, potentially requiring a larger audit.

The codesearch I used to determine that is:

https://codesearch.debian.net/search?q=filetype%3Ac+zipOpenNewFileInZip4_64&literal=0&perpkg=1

I haven't performed any callflow analysis to determine how many of those
packages make use of the affected zipOpenNewFileInZip4_64 function.

(I also accidentally omitted the security list from my previous comment, which
contains a patch based on the existing upstream fix)

[James Addison]

Source: zlib
Followup-For: Bug #1054290

Control: tags -1 patch

[James Addison]

Source: zlib
Followup-For: Bug #1054290

X-Debbugs-Cc: car...@debian.org, Debian Security Team <te...@security.debian.org>

After reading the minizip/zip.c code[1], I think that the vulnerable function
is exposed for external linkage by any of the 'zipOpenNewFile*' functions.

Given that, I code-searched[2] for 'zipOpenFile' and collected the resulting
'packages.txt' file list provided under the dropdown menu.

To retrieve the corresponding source packages _for trixie_ I used the command:

$ cat packages.txt | while read line; do apt source $line; done;

...and then to search for possible callsites, and report their filenames and
line numbers:

$ find . -type f -name '*.c*' -exec grep -Hn zipOpenNewFile {} \; | grep -vw "minizip/minizip.c" | grep -vw "minizip/zip.c"

(note that this also filters out the vendored source files themselves,
because otherwise those would pollute the search results)

I then manually removed 'chromium' from the results because that codebase has
the patch applied[3] already (this is from upstream I believe).

Finally I manually-removed a bunch of callsites that relate to error message
text, #defines, local variables and in the case of Mono, some external
dynamic library references.

It appears to me that 34 packages in Trixie may be affected, with a total
of 59 lines of callsites to 'zipOpenFile' functions. Please find my
assessment attached as 'trixie-callsites.txt'.

[1] - https://sources.debian.org/src/zlib/1%3A1.2.13.dfsg-1/contrib/minizip/zip.c/

[2] - https://codesearch.debian.net/search?q=zipOpenNewFile&perpkg=1

[3] - https://sources.debian.org/src/chromium/118.0.5993.70-1/third_party/zlib/contrib/minizip/zip.c/?hl=1327#L1086-L1095

[David Dooling]

Thanks for that analysis, James.

Using James' analysis as a starting point, I dug into some of the
usages of the 'zipOpenNewFile*' functions.

nodejs-18.13.0+dfsg1:
The Node.js source code includes a copy of the zlib source code. This
copy was patched over a month ago.

mariadb-10.11.4:
The MariaDB source code includes a copy of the zlib source code. This
copy has not been patched. This implies MariaDB should be mentioned in
this CVE but is not.

After checking two common packages and seeing the same, someone
nonstandard pattern, I downloaded and compiled zlib myself. By
default, it does not appear that any of the minizip functions are
included in any header file or library installed as part of the normal
zlib './configure && make && make install'. So perhaps all these
usages of these functions are associated with downstream software
closing zlib source into their code? If that is the case, what does
that mean for this CVE and actually creating a coherent response
across all these packages?

--
David Dooling

[James Addison]

Source: zlib
Followup-For: Bug #1054290

X-Debbugs-Cc: david.dool...@docker.com, car...@debian.org, Debian Security Team <te...@security.debian.org>

On Tue, 31 Oct 2023 13:13:00 -0500, David wrote:
> Thanks for that analysis, James.

...

> nodejs-18.13.0+dfsg1:
> The Node.js source code includes a copy of the zlib source code. This
> copy was patched over a month ago.

Thanks David for the follow-up! I had indeed missed that nodejs is already
patched; great -- and that also demonstrates that I hadn't checked whether
each callsite I found was using a locally-vendored copy of the minizip code
(that's the situation you mention where packages clone zlib into their own
code).

As context: Debian's preferred approach[1] is that each package de-duplicates
references to library code that is already packaged in Debian -- even if the
originating codebase simply includes a copy of it -- because when a security
issue like this appears, it's easier if there's only a single place to patch.

> After checking two common packages and seeing the same, someone
> nonstandard pattern, I downloaded and compiled zlib myself. By
> default, it does not appear that any of the minizip functions are
> included in any header file or library installed as part of the normal
> zlib './configure && make && make install'. So perhaps all these
> usages of these functions are associated with downstream software
> closing zlib source into their code? If that is the case, what does
> that mean for this CVE and actually creating a coherent response
> across all these packages?

About the default compile options for zlib: that's correct: minizip is
a non-core part of the zlib codebase, referred to as a 'contrib'[2], often
implying a lower maintenance guarantee. Debian does provide a package called
'minizip' that builds binaries from the relevant code, though.

About what to do regarding the other packages:

After removing nodejs, and checking each remaining package's source, I find
that only 11 (eleven) of the packages in fact contain a full copy of minizip
with the vulnerable section of code.

A few packages referenced 'quazip' - a fork of minizip. Of those, only
1 (one) appears to support 64-bit zip files, and it does look like it has
the vulnerability too.

For 3 (three) of the remaining packages, I'm uncertain whether copied code that
looks like older versions minizip is in fact vulnerable; those are the
'magics++' and 'widelands' packages, where 64-bit zip support appears
incomplete or missing, and 'gdal', where the code appears to be part of a
library called 'CPL' that may have shared some lineage with minizip.

In the latter case, I'll ask the CPL maintainers whether they think it's
affected on their mailing list[3].

The twelve (11 minizip + 1 quazip) vendored code locations are:

https://sources.debian.org/src/gmsh/4.8.4+ds2-3/contrib/zipper/zip.c/?hl=1055#L1085
https://sources.debian.org/src/godot/3.5.2-stable-2/thirdparty/minizip/zip.c/?hl=1292#L1088
https://sources.debian.org/src/httrack/3.49.4-1/src/minizip/zip.c/?hl=1311#L1086
https://sources.debian.org/src/libxlsxwriter/1.1.5-1/third_party/minizip/zip.c/?hl=1057#L1088
https://sources.debian.org/src/mariadb/1:10.11.5-3/storage/connect/zip.c/?hl=1337#L1086
https://sources.debian.org/src/mupen64plus-core/2.5.9+341+gf82b37bf-1/subprojects/minizip/zip.c/?hl=1265#L1086
https://sources.debian.org/src/orthanc/1.12.1+dfsg-4/OrthancFramework/Resources/ThirdParty/minizip/zip.c/?hl=1337#L1086
https://sources.debian.org/src/qt6-webengine/6.4.2-final+dfsg-12/src/3rdparty/chromium/third_party/zlib/contrib/minizip/zip.c/?hl=1273#L1086
https://sources.debian.org/src/qtwebengine-opensource-src/5.15.15+dfsg-2/src/3rdparty/chromium/third_party/zlib/contrib/minizip/zip.c/?hl=1288#L1086
https://sources.debian.org/src/rbdoom3bfg/1.4.0+dfsg-2/neo/libs/zlib/minizip/zip.cpp/?hl=1307#L1112
https://sources.debian.org/src/swi-prolog/9.0.4+dfsg-2/src/minizip/zip.c/?hl=1065#L1096
https://sources.debian.org/src/tea/62.0.2-1/zip.c/?hl=1345#L1113


I'd like someone else to confirm with me whether this is a good idea or not,
but my suggestion would be that we file a bug with each of the Debian source
packages (src:gmsh for example), linking to the affected line and referencing
this bug and the CVE to explain the problem.

Then each package's maintainer could check whether they think their package is
genuinely affected, and decide whether to patch the problem within Debian only,
and/or to forward the fix to the code distributors (aka upstream), or to do
nothing (which is a valid choice, especially if the code can't be reached from
their package even if it's theoretically built into it).

It's possible I'm overexplaining some of that, so my apologies if so - just
trying to be clear.


[1] - https://www.debian.org/doc/debian-policy/ch-source.html#embedded-code-copies

[2] - https://github.com/madler/zlib/issues/868#issuecomment-1783525536

[3] - https://lists.osgeo.org/mailman/listinfo/gdal-dev

[James Addison]

Source: zlib
Followup-For: Bug #1054290

X-Debbugs-Cc: david.dool...@docker.com, car...@debian.org, Debian Security Team <te...@security.debian.org>

On Fri, 03 Nov 2023 14:26:54 +0000, I wrote:
> A few packages referenced 'quazip' - a fork of minizip. Of those, only
> 1 (one) appears to support 64-bit zip files, and it does look like it has
> the vulnerability too.
>
> For 3 (three) of the remaining packages, I'm uncertain whether copied code that
> looks like older versions minizip is in fact vulnerable; those are the
> 'magics++' and 'widelands' packages, where 64-bit zip support appears
> incomplete or missing, and 'gdal', where the code appears to be part of a
> library called 'CPL' that may have shared some lineage with minizip.

Please note: both of those paragraphs I wrote mention 64-bit zipfile support,
because I thought that that could be a prerequisite for the vulnerability (an
integer overflow). However: I'm not really sure whether that's true or not.

[James Addison]

Source: zlib
Followup-For: Bug #1054290

I now think that patching vendored minizip code in libxlsxwriter would not help
because it specifies the 'USE_SYSTEM_MINIZIP' define at build-time[1] in
combination with a build-time dependency[2] on 'libminizip-dev' to link to the
required library functions.

In other words: if-and-when a security update is available in libminizip-dev
then libxlsxwriter will benefit from that automatically, and the presence of
apparently-vulnerable code within src:libxlsxwriter is irrelevant.

[1] - https://sources.debian.org/src/libxlsxwriter/1.1.5-1/debian/rules/#L14

[2] - https://sources.debian.org/src/libxlsxwriter/1.1.5-1/debian/control/#L11

[James Addison]

Followup-For: Bug #1054290

Updates on some other codebases where minizip appears vendored in Debian source
packages:

* gdal - the fix for minizip is included in upstream version 3.8.0 and a
packaged version of that release has been accepted into Debian unstable.

* mupen64plus-core - this appears unaffected in Debian; it declares a
build-time dependency on libminizip-dev, and the build system uses this
when available. I've verified that by recompiling it successfully with the
'minizip' subproject directory deleted from the filesystem.

* orthanc - appears unaffected due to the nature of the inputs to the
filename parameter.

* widelands - appears unaffected in Debian; similar to mupen64plus-core, it
declares a dependecy on libminizip-dev, and the build system for the
codebase uses system-provided minizip in preference to the vendored copy.

[James Addison]

Followup-For: Bug #1054290

Sorry, I made an important mistake in my phrasing about these two packages:

> * mupen64plus-core - this appears unaffected in Debian; it declares a
> build-time dependency on libminizip-dev, and the build system uses this
> when available. I've verified that by recompiling it successfully with the
> 'minizip' subproject directory deleted from the filesystem.

> * widelands - appears unaffected in Debian; similar to mupen64plus-core, it
> declares a dependecy on libminizip-dev, and the build system for the
> codebase uses system-provided minizip in preference to the vendored copy.

I should not have said that these are unaffected; they may be affected if the
system-provided minizip library is vulnerable.

Additionally, patching the vendored copy of minizip source code within those
packages alone would not help, again for the same reason that they use the
system-provided minizip.

This is the same condition as the libxlsxwriter case.