Re: Zlib memory corruption on deflate | Hacker News Reader

Re: Zlib memory corruption on deflate

openwall.com

53 points0x013 comments

This seems to be a minor bug, in that it is only triggered using unusual (and rather unlikely) deflate parameters.

But, looking into this bug, I was sort-of interested to see how it was handled. The change log for 1.2.12 (2022-03-27) indicates the issue was resolved ("Fix a bug that can crash deflate on some input when using Z_FIXED"). Yet, in what seems to be the canonical Zlib repository (https://github.com/madler/zlib), I was unable to find a corresponding commit.

None of commits this year (7, so not too hard to review) seem to be particularity meaningful changes, and in particular yesterday's 'zlib 1.2.12' commit seems to only consist of version/copyright updates.

So, does anyone have any idea where to find the commits related to the change log entry? (Note that I'm not disputing the issue is actually fixed, I'm just trying to improve my Github reading skills...)

It was fixed in 2018, but no release was made at the time https://github.com/madler/zlib/commit/5c44459c3b28a9bd3283aa...

Thank you very much! Sort-of surprising there is so much discussion about a long-fixed bug, yet, there we are...

A lot of older projects (especially statically linked Windows applications) might contain an older release of zlib which is still affected by the bug. Don't know anything about the nature of the bug, but if the corruption could be converted to memory corruption... we have a problem.

I wonder if FreePascal is affected

Looks like they ported zlib to Pascal in 1998 and left it pretty much unchanged:

https://gitlab.com/freepascal.org/fpc/source/-/blob/main/pac...

For lack of better avenues, I'll repeat this here: Don't use the hand-optimized assembly code that comes with zlib. It will read out of bounds given the right input data. By default, this affects the VS solutions, but please double-check your build. More info: https://news.ycombinator.com/item?id=30084771

what is more concerning is that the maintainer didn't understand this impact case, even when two people showed him the exact logic, linenumber and a reproducer. he was still the opinion that this cannot happen in production.

we need a zlib fork

memLevel=1 afaik is pretty rare in the wild? The maintainer doesn't seem to dispute it after a reproducer was shown, a new release is out now.

default memLevel is 8. Memory usage is basically defined by windowBits and memLevel; 1 would be really unusual, even for embedded scenarios.

Yeah, the existence of memLevel is basically a historical quirk. The total memory used by zlib at default memLevel and windowBits is under 300KB (it needs one 256KB buffer and a few small additional allocations). There's not a lot of environments left where it makes sense to run zlib where that's too much.

I showed him the exact problematic line, but he didn't seem to understand it. same thing happened previously.

There are already active zlib forks (e.g. https://github.com/zlib-ng/zlib-ng), the problem is with having people move to them. It takes a lot of effort to move mindshare from the original version to a fork, there's some historical examples of it happening, but not a ton.

ah, thanks. this looks indeed as not from the last century. much better code.

Reply on news.ycombinator.com