Re: CVE-2017-14103 / graphicsmagick

[Brian May <bam@debian.org>]

Brian May <brian@linuxpenguins.xyz> writes:

> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0
> read(3, "", 4096)                       = 0

The default logging printed %c bytes that were currupted and prevented
seeing the length. I made the following change:

=== cut ===
--- graphicsmagick-1.3.16.orig/coders/png.c
+++ graphicsmagick-1.3.16/coders/png.c
@@ -3108,8 +3108,8 @@ static Image *ReadOneJNGImage(MngInfo *m
 
       if (logging)
         (void) LogMagickEvent(CoderEvent,GetMagickModule(),
-                              "  Reading JNG chunk type %c%c%c%c, length: %lu",
-                              type[0],type[1],type[2],type[3],length);
+                              "  Reading JNG chunk type, length: %lu",
+                              length);
 
       if (length > PNG_MAX_UINT || count == 0)
         {
=== cut ===

Now I can see the length:

=== cut ===
07:39:12 0:01 0.000u 21326 png.c/ReadOneJNGImage/3110/Coder:
    Reading JNG chunk type, length: 336592896
=== cut ===

Looks like we want to read every single byte even though the underlying
sys call is not returning any data. As proven with another change:

=== cut ===
--- graphicsmagick-1.3.16.orig/coders/png.c
+++ graphicsmagick-1.3.16/coders/png.c
@@ -3129,8 +3129,10 @@ static Image *ReadOneJNGImage(MngInfo *m
                   "    Could not allocate chunk memory");
               return ((Image*)NULL);
             }
-          for (i=0; i < (long) length; i++)
+          for (i=0; i < (long) length; i++) {
+            (void) LogMagickEvent(CoderEvent,GetMagickModule(), "reading chunk (%lu)",i);
             chunk[i]=ReadBlobByte(image);
+          }
           p=chunk;
         }
       (void) ReadBlobMSBLong(image);  /* read crc word */
=== cut ===

Now I get messages like:

=== cut ===
..
07:43:23 0:18 14.640u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (418921)
07:43:23 0:18 14.640u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (418922)
07:43:23 0:18 14.640u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (418923)
07:43:23 0:18 14.640u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (418924)
...
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871661)
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871662)
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871663)
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871664)
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871665)
07:44:31 1:26 62.960u 21408 png.c/ReadOneJNGImage/3133/Coder:
  reading chunk (1871666)
...
=== cut ===

I imagine this could be considered another security issue, being a DOS
attack.

Going to look through the CVEs again, don't think I noticed anything
like this.
-- 
Brian May <bam@debian.org>