The vulnerability in question is the XV input-validation one from last August or so; Gentoo, SuSE, OpenBSD, and undoubtedly other vendors all put together various patches addressing the problem to varying extents. The vulnerability (or the set of them, really) was brought to light in a BugTraq posting that included a (supposedly) working exploit for one of them: http://www.securityfocus.com/archive/1/372345 The problem is that the additional checks considered only single-factor or two-factor overflows--that is, are height and width reasonable? Is their product positive? For example, the SuSE/Gentoo patch includes this fragment (from http://bugs.gentoo.org/show_bug.cgi?id=61619): --- xvpcx.c +++ xvpcx.c Tue Aug 24 13:12:15 2004 @@ -222,7 +222,14 @@ byte *image; /* note: overallocation to make life easier... */ - image = (byte *) malloc((size_t) (pinfo->h + 1) * pinfo->w + 16); + int count = (pinfo->h + 1) * pinfo->w + 16; + + if (count <= 0 || pinfo->h <= 0 || pinfo->w <= 0) { + pcxError(fname, "Bogus PCX file!!"); + return (0); + } + + image = (byte *) malloc((size_t) count); if (!image) FatalError("Can't alloc 'image' in pcxLoadImage8()"); xvbzero((char *) image, (size_t) ((pinfo->h+1) * pinfo->w + 16)); (This is in the 8-bit code.) Because of the additive factors, count can be as large as 4295032848 == 65552 on machines with 32-bit integers; obviously that's positive. Setting the height to 65536 and the width to 65535 requires only 15 bytes of "fill" before the heap-overflowing exploit code can begin. The more general case is in the 24-bit code; this is the one that is most likely to affect all other 24-bit formats, too: @@ -250,17 +257,25 @@ { byte *pix, *pic24, scale[256]; int c, i, j, w, h, maxv, cnt, planes, bperlin, nbytes; + int count; w = pinfo->w; h = pinfo->h; planes = (int) hdr[PCX_PLANES]; bperlin = hdr[PCX_BPRL] + ((int) hdr[PCX_BPRH]<<8); + count = w*h*planes; + + if (count <= 0 || planes <= 0 || w <= 0 || h <= 0) { + pcxError(fname, "Bogus PCX file!!"); + return (0); + } + /* allocate 24-bit image */ - pic24 = (byte *) malloc((size_t) w*h*planes); + pic24 = (byte *) malloc((size_t) count); if (!pic24) FatalError("couldn't malloc 'pic24'"); - xvbzero((char *) pic24, (size_t) w*h*planes); + xvbzero((char *) pic24, (size_t) count); maxv = 0; pix = pinfo->pic = pic24; In principle, "planes" can be as large as 255, but this function isn't reached unless it's exactly equal to 3. That's more than enough when w and h can each be as large as 65536, however. My PCX demo images, which contain enough information to allow a clueful attacker to spread the joy to other image formats, have unfortunately been attached to this KDE bug: http://bugs.kde.org/show_bug.cgi?id=102328 (I moved the images on my site before anyone else downloaded them-- at least, assuming my ISP's web logs can be trusted--so those links are now broken. But the attachment, id 10321, is officially Out There, and I have no control over it.) Note that, despite the implication in the KDE bug report, this is not limited to the PCX format--in XV, it also affects at least xvpm.c and xvpng.c, and almost certainly xvjpeg.c and xvtiff.c as well. Nor is it necessarily limited to corrupted images; one could create a perfectly valid 4 GB PCX image whose pixel data just happened to be interpretable as executable code, though it's unlikely to be an effective trojan. It is also not limited solely to XV and gwenview. Bruno reported that ImageMagick's convert suffers some sort of memory over-allocation problem (a DoS, at least), and IrfanView (Win32) crashes. He found Imlib-based decoders seem to be immune, as does the GIMP (not sure if it's based on Imlib). In general, any 24-bit image decoder that allocates a single buffer to hold the entire image may be vulnerable. Here's my patch for xvpcx.c and xvpm.c (against the original XV 3.10a sources). I haven't yet had time to make similar fixes for the PNG decoder or any others, and the patch includes some extra whitespace fluff since it's extracted from my jumbo-bugfix patch. (Sorry.) A couple of the comments may differ slightly from what I sent to CERT. diff -ru xv-3.10a/xvpcx.c xv-3.10a-bugfixes/xvpcx.c --- xv-3.10a/xvpcx.c 1995-01-10 15:06:37.000000000 -0800 +++ xv-3.10a-bugfixes/xvpcx.c 2005-03-20 20:45:00.000000000 -0800 @@ -218,15 +218,21 @@ byte *hdr; { /* load an image with at most 8 bits per pixel */ - + byte *image; - + int count; + /* note: overallocation to make life easier... */ - image = (byte *) malloc((size_t) (pinfo->h + 1) * pinfo->w + 16); + count = (pinfo->h + 1) * pinfo->w + 16; /* 4295032848 max == 65552 (wrap) */ + if (pinfo->w <= 0 || pinfo->h <= 0 || count/pinfo->w < pinfo->h) { + pcxError(fname, "Bogus PCX file!!"); + return (0); + } + image = (byte *) malloc((size_t) count); if (!image) FatalError("Can't alloc 'image' in pcxLoadImage8()"); - - xvbzero((char *) image, (size_t) ((pinfo->h+1) * pinfo->w + 16)); - + + xvbzero((char *) image, (size_t) count); + switch (hdr[PCX_BPP]) { case 1: pcxLoadRaster(fp, image, 1, hdr, pinfo->w, pinfo->h); break; case 8: pcxLoadRaster(fp, image, 8, hdr, pinfo->w, pinfo->h); break; @@ -249,25 +255,33 @@ byte *hdr; { byte *pix, *pic24, scale[256]; - int c, i, j, w, h, maxv, cnt, planes, bperlin, nbytes; - + int c, i, j, w, h, maxv, cnt, planes, bperlin, nbytes, count; + w = pinfo->w; h = pinfo->h; - - planes = (int) hdr[PCX_PLANES]; - bperlin = hdr[PCX_BPRL] + ((int) hdr[PCX_BPRH]<<8); - + + planes = (int) hdr[PCX_PLANES]; /* 255 max */ + bperlin = hdr[PCX_BPRL] + ((int) hdr[PCX_BPRH]<<8); /* 65535 max */ + + j = h*planes; /* w and h are limited to 65536, planes to 3 */ + count = w*j; /* ...so this could wrap up to 3 times */ + nbytes = bperlin*j; /* ...and this almost 3 times */ + if (w <= 0 || h <= 0 || planes <= 0 || bperlin <= 0 || + j/h < planes || count/w < j || nbytes/bperlin < j) { + pcxError(fname, "Bogus PCX file!!"); + return (0); + } + /* allocate 24-bit image */ - pic24 = (byte *) malloc((size_t) w*h*planes); - if (!pic24) FatalError("couldn't malloc 'pic24'"); - - xvbzero((char *) pic24, (size_t) w*h*planes); - + pic24 = (byte *) malloc((size_t) count); + if (!pic24) FatalError("Can't malloc 'pic24' in pcxLoadImage24()"); + + xvbzero((char *) pic24, (size_t) count); + maxv = 0; pix = pinfo->pic = pic24; i = 0; /* planes, in this while loop */ j = 0; /* bytes per line, in this while loop */ - nbytes = bperlin*h*planes; - + while (nbytes > 0 && (c = getc(fp)) != EOF) { if ((c & 0xC0) == 0xC0) { /* have a rep. count */ cnt = c & 0x3F; diff -ru xv-3.10a/xvpm.c xv-3.10a-bugfixes/xvpm.c --- xv-3.10a/xvpm.c 1994-12-22 14:34:40.000000000 -0800 +++ xv-3.10a-bugfixes/xvpm.c 2005-03-20 22:16:52.000000000 -0800 @@ -60,7 +60,7 @@ FILE *fp; byte *pic8; - int isize,i,flipit,w,h; + int isize,i,flipit,w,h,numbytes; char *bname; bname = BaseName(fname); @@ -114,20 +115,27 @@ fprintf(stderr,"(ie, 1-plane PM_I, or 1-, 3-, or 4-plane PM_C)\n"); return pmError(bname, "PM file in unsupported format"); - } + } isize = pm_isize(&thePic); + i = w*h; + numbytes = i*3; + + /* make sure image is more-or-less valid (and no overflows) */ + if (isize <= 0 || w <= 0 || h <= 0 || i/w < h || numbytes/i < 3 || + thePic.pm_cmtsize < 0) + return pmError(bname, "Bogus PM file!!"); - if (DEBUG) + if (DEBUG) fprintf(stderr,"%s: LoadPM() - loading a %dx%d %s pic, %d planes\n", - cmd, w, h, (thePic.pm_form==PM_I) ? "PM_I" : "PM_C", + cmd, w, h, (thePic.pm_form==PM_I) ? "PM_I" : "PM_C", thePic.pm_np); - + /* allocate memory for picture and read it in */ thePic.pm_image = (char *) malloc((size_t) isize); - if (thePic.pm_image == NULL) + if (thePic.pm_image == NULL) return( pmError(bname, "unable to malloc PM picture") ); if (fread(thePic.pm_image, (size_t) isize, (size_t) 1, fp) != 1) {
Follow up comment by reporter on Vendor-Sec: I think we can consider this vulnerability to be "effectively exploited" in the sense that an exploit for an earlier version of the same problem in XV's BMP decoder was posted to BugTraq last August (http://www.securityfocus.com/archive/1/372345), and demonstration images for the new variant are publicly available from the KDE Bugzilla site (http://bugs.kde.org/show_bug.cgi?id=102328). I'm pretty sure the former can be trivially adapted to the latter, though I have not attempted to do so myself. Ergo, I still plan to release an updated set of XV jumbo patches tonight or tomorrow morning (US/Pacific) and to make an announcement to BugTraq within the next day or two. I realize this is a holiday weekend for many, and that makes things awkward, but unfortunately it doesn't alter anything I said in the previous paragraph. In the meantime, here are some updated test images: http://pobox.com/~newt/test/286572/overflow-examples.zip (189695 bytes) http://pobox.com/~newt/test/286572/normal-examples.zip (189638 bytes) (I trust no one will post the new links on publicly visible bug pages just yet! :-/ ) The archives contain the same 8-bit PCX image as in the KDE bug attachment, plus 24-bit BMP, JPEG, PCX (slightly "improved"), PNG, PPM, and TIFF versions. All but the PNG trigger segfaults in XV: % foreach j ( overflow-[28]* ) foreach? echo $j foreach? /usr/X11R6/bin/xv $j foreach? end overflow-24.bmp Segmentation fault overflow-24.jpg Segmentation fault overflow-24.pcx Segmentation fault overflow-24.png overflow-24.ppm Segmentation fault overflow-24.tif Segmentation fault overflow-8.pcx Segmentation fault (The PNG decoder is saved by internal libpng checks that apparently go all the way back to 1.0.8, maybe even earlier. On the other hand, a different but related libpng vulnerability was fixed just last August, so don't assume a PNG crack is entirely out of the question.) Note that I'm limiting my attention solely to XV, simply because it's the image viewer I know and love^Wuse. Hopefully most modern ones are a bit more secure.
Another followup: Unfortunately (or fortunately, depending on your perspective), I still have another 200 memory-allocations to inspect and potentially fix in XV, which means I'm not ready with my own patch and probably won't be before next weekend sometime. In particular, I won't be announcing anything for at least that long; I'd like the fix to be completely ready first.
Closed, public followup in bug 88742 *** This bug has been marked as a duplicate of 88742 ***
