search

madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.58k stars 2.43k forks source link

Ensure possible padding bytes are initialized #1002

Closed qarni closed 1 month ago

qarni commented 1 month ago

Microsoft's static analysis tool found a vulnerability from possible info leakage from uninitialized padding bytes in this struct, which I changed in microsoft/cmake. Pushing this change upstream so it can be added to kitware/cmake.

madler commented 1 month ago

Please provide the output of the static analysis tool. Thanks.

qarni commented 1 month ago 
Utilities/cmzlib/gzlib.c#L113 Memory allocation of [struct <unnamed>] /Utilities/cmzlib/gzguts.h#L170 includes uninitialized padding bytes. 

Possible information leakage from uninitialized padding bytes.
A newly allocated struct or class that is initialized member-by-member may leak information if it includes padding bytes.
Recommendation: Make sure that all padding bytes in the struct or class are initialized.

If possible, use memset to initialize the whole structure/class.

(Line numbers are a little off since kitware/cmake hasn't pulled in the newest version of zlib in a bit)

madler commented 1 month ago

Thanks. What version of zlib was this run on?

qarni commented 1 month ago

zlib 1.2.13

madler commented 1 month ago

According to this document: https://wiki.sei.cmu.edu/confluence/display/c/DCL39-C.+Avoid+information+leakage+when+passing+a+structure+across+a+trust+boundary , Microsoft's suggested remedy of using memset() does not in fact solve the potential security issue, shown there as a "noncompliant code example".

In any case, the issue is an issue only if the structure is being passed across a trust boundary, e.g., out of the kernel. zlib does no such passing. The structure in question is internal to zlib and not accessible to the application through the zlib interface. zlib provides no facility to copy a gzFile object. In order for the application to copy a gzFile object, to get it across one of these trust boundaries, it would need to use gzguts.h to even know the size of the internal structure. Once an application is getting into zlib's pants, then all bets are off, and it is up to the application to guard against any security issues it may create. In that case, the correct remedy in the linked document is to copy the structure element-by-element. Since the application is already using gzguts.h, it has the information it needs to do that.

Thank you for your report. Nothing for zlib here. I am closing this issue.