processone/zlib

Fix zlib driver crashing on 1024-bytes chunks

Opened this issue · 1 comments

As reported by syhpoon:

We have found an issue in our production ejabberd node. Upon stream compression has been initiated any incoming stanza which is exactly 1024 bytes long would crash the zlib driver and connection would terminate. The problem is the following:

in apps/ejabberd/c_src/ejabberd_zlib_drv.c file internal buffer size is defined to be equal 1024 bytes.
In inflate logic one edge case is not properly handled, namely when original chunk (before deflated) is exactly BUF_SIZE bytes long.
Internal loop: while (err == Z_OK && d->i_stream->avail_out == 0) checks for the avail_out field to be greater than zero, however when original chunk is exactly BUF_SIZE bytes avail_zero is 0 because the output buffer was completely filled by inflating the chunk. So code enters the loop one extra time despite the fact that we do not have any more data to process. zlib in this case returns Z_BUF_ERROR to indicate exactly the situation we have (http://www.zlib.net/zlib_faq.html#faq05) and current code just checks for return code to be either Z_OK or Z_STREAM_END, otherwise it throws an error.

Quote on this problem from http://www.zlib.net/zlib_how.html:

The way we tell that deflate() has no more output is by seeing that it did not fill the output buffer, leaving avail_out greater than zero. However suppose that deflate() has no more output, but just so happened to exactly fill the output buffer! avail_out is zero, and we can't tell that deflate() has done all it can. As far as we know, deflate() has more output for us. So we call it again. But now deflate() produces no output at all, and avail_out remains unchanged as CHUNK. That deflate() call wasn't able to do anything, either consume input or produce output, and so it returns Z_BUF_ERROR. (See, I told you I'd cover this later.) However this is not a problem at all. Now we finally have the desired indication that deflate() is really done, and so we drop out of the inner loop to provide more input to deflate().

For deflate case this problem is the opposite - it would crash if the compressed chunk is exactly 1024 bytes long.

Should be fixed by e3d4222