Re: CVE-2017-14103 / graphicsmagick
Ok, I think I fixed both problems in wheezy:
(wheezy-i386-default)root@prune:/tmp/brian/tmpWzJq5p/build/i386# gm convert /tmp/10.crashes.png /tmp/abc2.png
gm convert: Request did not return an image.
With two patches, first one is a change I copied from the wheezy version
to make it read more efficiently (was reading one byte at a time
although this is buffered closer to the syscall) and to stop reading on
EOF:
=== cut ===
--- graphicsmagick-1.3.16.orig/coders/png.c
+++ graphicsmagick-1.3.16/coders/png.c
@@ -3121,8 +3121,18 @@ static Image *ReadOneJNGImage(MngInfo *m
if (chunk == (unsigned char *) NULL)
ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
image);
- for (i=0; i < (long) length; i++)
- chunk[i]=ReadBlobByte(image);
+ if (ReadBlob(image,length,chunk) < length)
+ {
+ if (color_image_info != (ImageInfo *)NULL)
+ {
+ DestroyImageInfo(color_image_info);
+ }
+ if (alpha_image_info != (ImageInfo *)NULL)
+ {
+ DestroyImageInfo(alpha_image_info);
+ }
+ ThrowReaderException(CorruptImageError,CorruptImage,image);
+ }
p=chunk;
}
(void) ReadBlobMSBLong(image); /* read crc word */
=== cut ===
Second change is ported from the upstream fix (note it also covers the
above code too). Apparently throwing an exception (???) is a problem, so
we log the error and return NULL instead. In later versions there is
code to free the memory used too, a change I did not try to copy.
=== cut ===
--- a/coders/png.c
+++ b/coders/png.c
@@ -3112,15 +3112,23 @@
type[0],type[1],type[2],type[3],length);
if (length > PNG_MAX_UINT || count == 0)
- ThrowReaderException(CorruptImageError,CorruptImage,image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ "chunk length (%lu) > PNG_MAX_UINT",length);
+ return ((Image*)NULL);
+ }
+
chunk=(unsigned char *) NULL;
p=NULL;
if (length)
{
chunk=MagickAllocateMemory(unsigned char *,length);
if (chunk == (unsigned char *) NULL)
- ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
- image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " Could not allocate chunk memory");
+ return ((Image*)NULL);
+ }
if (ReadBlob(image,length,chunk) < length)
{
if (color_image_info != (ImageInfo *)NULL)
@@ -3131,7 +3139,9 @@
{
DestroyImageInfo(alpha_image_info);
}
- ThrowReaderException(CorruptImageError,CorruptImage,image);
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " chunk reading was incomplete");
+ return ((Image*)NULL);
}
p=chunk;
}
@@ -3214,14 +3224,19 @@
color_image_info=MagickAllocateMemory(ImageInfo *,sizeof(ImageInfo));
if (color_image_info == (ImageInfo *) NULL)
- ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
- image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not allocate color_image_info");
+ return ((Image *)NULL);
+ }
GetImageInfo(color_image_info);
color_image=AllocateImage(color_image_info);
if (color_image == (Image *) NULL)
- ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
- image);
-
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not allocate color_image");
+ return ((Image *)NULL);
+ }
if (logging)
(void) LogMagickEvent(CoderEvent,GetMagickModule(),
" Creating color_blob.");
@@ -3229,23 +3244,31 @@
status=OpenBlob(color_image_info,color_image,WriteBinaryBlobMode,
exception);
if (status == MagickFalse)
- ThrowReaderException(CoderError,UnableToOpenBlob,color_image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not open color_image blob");
+ return ((Image *)NULL);
+ }
+
if (!image_info->ping && jng_color_type >= 12)
{
alpha_image_info=MagickAllocateMemory(ImageInfo *,
sizeof(ImageInfo));
if (alpha_image_info == (ImageInfo *) NULL)
- ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
- image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not allocate alpha_image_info");
+ return ((Image *)NULL);
+ }
GetImageInfo(alpha_image_info);
alpha_image=AllocateImage(alpha_image_info);
if (alpha_image == (Image *) NULL)
{
DestroyImage(alpha_image);
- ThrowReaderException(ResourceLimitError,
- MemoryAllocationFailed,
- alpha_image);
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not allocate alpha_image");
+ return ((Image *)NULL);
}
if (logging)
(void) LogMagickEvent(CoderEvent,GetMagickModule(),
@@ -3254,7 +3277,11 @@
status=OpenBlob(alpha_image_info,alpha_image,WriteBinaryBlobMode,
exception);
if (status == MagickFalse)
- ThrowReaderException(CoderError,UnableToOpenBlob,image);
+ {
+ (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+ " could not open alpha_image blob");
+ return ((Image *)NULL);
+ }
if (jng_alpha_compression_method == 0)
{
unsigned char
@@ -3324,8 +3351,7 @@
(void) WriteBlobMSBULong(alpha_image,
crc32(crc32(0,data,4),chunk,length));
}
- if (length)
- MagickFreeMemory(chunk);
+ MagickFreeMemory(chunk);
continue;
}
=== cut ===
--
Brian May <bam@debian.org>
Reply to: