Description of problem: M. Joonas Pihlaja discovered security flaws in GraphicsMagick that also affect ImageMagick -- one possible buffer overflow in coders/dcm.c:ReadDCMImage() and three possible heap overflows in coders/palm.c:ReadPALMImage(). Debian project includes a fix for GraphicsMagick 1.1.7 among other changes in their patch. Version-Release number of selected component (if applicable): How reproducible: Potentially exploitable by maliciously crafted image. Fix: I attach the relevant part of the debian patch. It doesn't apply against ImageMagick without modifications, because GraphicMagics project uses different coding style. The patch needs to be reviewed and eventually needs to be rewritten.
Created attachment 138585 [details] Relevan excerpt from a Debian patch
Picked up and have ported the patch into ImageMagick and done a test build locally. Will be looking into which all releases are impacted tomorrow.
Suse has informed us that there is a bug in this patch, here is the corrected chunk: @@ -399,6 +399,7 @@ for (i=0; i < (long) bytes_per_row; ) { count=ReadBlobByte(image); + count=Min(count, bytes_per_row-i); byte=ReadBlobByte(image); (void) ResetMagickMemory(one_row+i,(int) byte,count); i+=count;
This bug ticket was used as a reference for a Fedora Core 5 fix for the CVE-2006-5456 issue in ImageMagick. Because of comment #5, I verified the patch used for FC-5 for this issue, at this URL: http://cvs.fedora.redhat.com/viewcvs/rpms/ImageMagick/FC-5/ImageMagick-6.2.5-cve-2006-5456.patch?sortby=date&view=markup It looks like the section mentioned in comment #5 is fine in FC-5's patch. Am I missing something, Josh, or is it just in a proposed patch for an RHEL package for which the issue in comment #5 is relevant?
I have to disagree with the updated chunk as it is possible to reach that section of code with count uninitialized which would then result in an infinite loop. if (compressionType == PALM_COMPRESSION_RLE) { image->compression=RLECompression; for (i=0; i < (long) bytes_per_row; ) { count=Min(ReadBlobByte(image),bytes_per_row-i); byte=ReadBlobByte(image); (void) ResetMagickMemory(one_row+i,(int) byte,count); i+=count; } } being the whole segment. If count == 0 at the beginning of that loop, then one will never get out of it. Additionally upstream maintains (in current svn) something closer to the original chunk: if (compressionType == PALM_COMPRESSION_RLE) { /* TODO move out of loop! */ image->compression=RLECompression; for (i=0; i < (long) bytes_per_row; ) { count=(ssize_t) Min(ReadBlobByte(image),(long) bytes_per_row-i); byte=(unsigned long) ReadBlobByte(image); (void) ResetMagickMemory(one_row+i,(int) byte,(size_t) count); i+=count; } }
The correct patch got CVE-2007-0770
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2007-0015.html