Re: [xml] libxml violates the zlib interface and crashes

[Daniel Veillard <veillard redhat com>]

  Hi Mark !

On Mon, Jan 18, 2010 at 01:49:30AM -0800, Mark Adler wrote:
> libxml authors and/or maintainers,

> I am preparing a new release of zlib which includes a rewrite of the gz* functions (to wit, that read and 
> write gzip files).  In the process of testing, Mark Brown discovered that libxml encounters a segmentation 
> fault with the current beta versions of zlib (1.2.3.5 and later).  Within libxml version 2.7.6 in xmlIO.c 
> we find this offending code:
>
> #ifdef HAVE_ZLIB_H
>       if ((xmlInputCallbackTable[i].opencallback == xmlGzfileOpen) &&
>               (strcmp(URI, "-") != 0)) {
>           if (((z_stream *)context)->avail_in > 4) {
>               char *cptr, buff4[4];
>               cptr = (char *) ((z_stream *)context)->next_in;
>               if (gzread(context, buff4, 4) == 4) {
>                   if (strncmp(buff4, cptr, 4) == 0)
>                       ret->compressed = 0;
>                   else
>                       ret->compressed = 1;
>                   gzrewind(context);
>               }
>           }
>       }
> #endif

  Argh, ...

> This appears to be an attempt to determine if the input is a gzip file
> or not.  It does so by finding and comparing bytes from an internal
> zlib buffer to the data returned by zlib, using "(z_stream *)context"
> to access the internal buffer, where "context" is zlib's otherwise opaque
> gzFile structure.

  yes I see bad case of API abuse, this need fixing obviously

> Since both libxml and zlib are widely deployed, and since I expect that applications usually link to it and 
> zlib dynamically, we have a problem.

  agreed !

> The fix to libxml is to check the version of zlib, and to use the
> current code for ZLIB_VERNUM less than 0x1230, and to instead use
> gzdirect() for ZLIB_VERNUM greater than or equal to 0x1230.  However,
> older versions of libxml without this fix will fail with newer versions
> of zlib.  E.g. if zlib is updated and libxml is not.


> --- xmlIO-2.7.6.c     2009-09-24 08:32:00.000000000 -0700
> +++ xmlIO.c   2010-01-18 01:30:27.000000000 -0800
> @@ -2518,6 +2518,9 @@
>  #ifdef HAVE_ZLIB_H
>       if ((xmlInputCallbackTable[i].opencallback == xmlGzfileOpen) &&
>               (strcmp(URI, "-") != 0)) {
> +#if defined(ZLIB_VERNUM) && ZLIB_VERNUM >= 0x1230
> +        ret->compressed = !gzdirect(context);
> +#else
>           if (((z_stream *)context)->avail_in > 4) {
>               char *cptr, buff4[4];
>               cptr = (char *) ((z_stream *)context)->next_in;
> @@ -2529,6 +2532,7 @@
>                   gzrewind(context);
>               }
>           }
> +#endif
>       }
>  #endif
>      }

  The patch seems to work for me on with zlib-1.2.3-23.fc12
paphio:~/XML -> grep ZLIB_VERNUM /usr/include/zlib.h 
#define ZLIB_VERNUM 0x1230
paphio:~/XML -> cat tst.xml.gz | xmllint -
<?xml version="1.0"?>
<foo/>
paphio:~/XML ->

  after a rebuild. Since the old cod is the same it should be safe too
so I'm pushing that patch.

  Now the real problem is if people upgrade gzip without pushing that
patch ... urgh

   Thanks for raising the issue and for the patch !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/