Advisory and Patch: Heap corruption in CVS

[Jeff Downs]

Greetings,

I have discovered a bug in CVS's interface to the zlib compression 
library. This bug results in CVS (server, maybe also client) accessing 
and writing memory that is outside the allocated bounds.

The memory in question is on the heap, so a simple buffer overflow is not 
a risk here.


This behavior was observed on a CVS pserver under Linux. Certain files 
would cause the pserver to seg fault.  After some debugging, it was found 
that the crash was happening during a call to free() which is made on 
internal data structures in zlib.  After further debugging, the cause was 
traced back to the zlib interface functions read_and_gzip() and 
gunzip_and_write() (in src/zlib.c).

These functions use pointer arithmetic and array indexing to access 
buffers on the heap without properly keeping track of the bounds of the 
buffer. Oddly, it seems like this bug has been in CVS for quite some time.


This bug will NEVER manifest itself with modern CVS clients as distributed 
in the cvs source package; these functions are only used in the 'gzip file 
contents' functionality of the client/server protocol. The current CVS 
clients from the cvshome.org source use 'gzip stream', which does not 
utilize this code. I did not audit the gzip stream functions for similar 
bugs.

This bug WILL manifest itself in a variety of ways (as is typical of heap 
corruption bugs) when using older CVS clients from the main cvs source ( <1.8
used gzip file contents I believe) and when using any third party CVS 
client which utilizes the gzip file contents functions.

We encountered this bug while using the CVS client built into the Netbeans 
IDE (www.netbeans.org). Netbeans' CVS client uses gzip file contents by 
default (this can be turned off as a workaround for anyone seeing this 
problem).

For this problem to become apparent in read_and_gzip, it must compress in
such a way that the output buffer (which is grown geometrically during
compression) has less than 8 bytes remaining when the trailing gzip CRC
data is written.  This scenario is very rare, but it does happen.

gunzip_and_write is in a far worse state - it blindly starts accessing the 
passed-in buffer to interpret the header information. This could cause 
problems if the gzip stream was truncated for any reason (intentionally by 
an attacker, or just by network interruption etc.).  There were also 
several spots where a strlen() call is made on a buffer of unknown 
content. If the buffer was malformed and did not contain a null byte, the 
strlen will continue beyond the bounds of the buffer.

Attached is a patch which demonstrates one possible fix for these two 
functions. These problems were seen with (and the patch generated against) 
RedHat Linux's cvs 1.11.2-10. A glance at 1.11.11's version of zlib.c 
indicates that the problem is still present.

This bug seems to have been observed by at least one other; 
issue #85 of cvshome.org's issue tracking for CCVS seems to also be 
related to these problems. The reporter's comments and insight are in 
agreement with the problems I am observing here. The reporting seems to be 
observing the same bug manifested in a different way (probably because he 
is running Solaris vs. Linux here).

Any replies should be CC'd to me, as I do not subscribe to this list.

        -Jeff

cvs-zlib-heapmgmnt.diff
Description: heap corruption fix