JNX: Fix DOS issues
authorBob Friesenhahn <bfriesen@GraphicsMagick.org>
Sat, 26 Aug 2017 14:14:13 -0500
changeset 15134 b037d79b6ccd
parent 15133 198ea602ea7c
child 15135 233a720bfd5e
JNX: Fix DOS issues
ChangeLog
VisualMagick/installer/inc/version.isx
coders/jnx.c
magick/version.h
www/Changelog.html
--- a/ChangeLog	Tue Aug 22 08:08:30 2017 -0500
+++ b/ChangeLog	Sat Aug 26 14:14:13 2017 -0500
@@ -1,3 +1,11 @@
+2017-08-26  Bob Friesenhahn  <bfriesen@simple.dallas.tx.us>
+
+	* coders/jnx.c (ReadJNXImage): Fix denial of service (DOS) issue
+	in ReadJNXImage() whereby large amounts of CPU and memory
+	resources may be consumed although the file itself does not
+	support the requests.  Problem was reported via email on Wed Aug
+	23 2017 by Xiaohei and Wangchu from Alibaba Security Team.
+
 2017-08-14  Glenn Randers-Pehrson  <glennrp@simple.dallas.tx.us>
 
 	* coders/png.c (ReadOneMNGImage): Deal with invalid (too large)
--- a/VisualMagick/installer/inc/version.isx	Tue Aug 22 08:08:30 2017 -0500
+++ b/VisualMagick/installer/inc/version.isx	Sat Aug 26 14:14:13 2017 -0500
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020170814"
-#define public MagickPackageReleaseDate "snapshot-20170814"
+#define public MagickPackageVersionAddendum ".020170826"
+#define public MagickPackageReleaseDate "snapshot-20170826"
--- a/coders/jnx.c	Tue Aug 22 08:08:30 2017 -0500
+++ b/coders/jnx.c	Sat Aug 26 14:14:13 2017 -0500
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2012-2015 GraphicsMagick Group
+% Copyright (C) 2012-2017 GraphicsMagick Group
 %
 % This program is covered by multiple licenses, which are described in
 % Copyright.txt. You should have received a copy of Copyright.txt with this
@@ -100,6 +100,7 @@
 
   char img_label_str[MaxTextExtent];
 
+
   alloc_size = TileInfo->PicSize + 2;
 
   if (image->logging)
@@ -242,6 +243,9 @@
     total_tiles,
     current_tile;
 
+  magick_off_t
+    file_size;
+
   /* Open image file. */
   assert(image_info != (const ImageInfo *) NULL);
   assert(image_info->signature == MagickSignature);
@@ -254,9 +258,8 @@
   if (status == False)
     ThrowReaderException(FileOpenError, UnableToOpenFile, image);
 
-  memset(JNXLevelInfo, 0, sizeof(JNXLevelInfo));
-
   /* Read JNX image header. */
+  (void) memset(&JNXHeader, 0, sizeof(JNXHeader));
   JNXHeader.Version = ReadBlobLSBLong(image);
   if (JNXHeader.Version > 4)
     ThrowReaderException(CorruptImageError, ImproperImageHeader, image);
@@ -266,8 +269,6 @@
   JNXHeader.MapBounds.SouthWest.lat = ReadBlobLSBLong(image);
   JNXHeader.MapBounds.SouthWest.lon = ReadBlobLSBLong(image);
   JNXHeader.Levels = ReadBlobLSBLong(image);
-  if (JNXHeader.Levels > 20)
-    ThrowReaderException(CorruptImageError, ImproperImageHeader, image);
   JNXHeader.Expiration = ReadBlobLSBLong(image);
   JNXHeader.ProductID = ReadBlobLSBLong(image);
   JNXHeader.CRC = ReadBlobLSBLong(image);
@@ -279,7 +280,41 @@
   if (EOFBlob(image))
     ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
 
+  file_size = GetBlobSize(image);
+
+  (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                        "JNX Header:\n"
+                        "    Version:    %u\n"
+                        "    DeviceSN:   %u\n"
+                        "    MapBounds:\n"
+                        "      NorthEast: lat = %u, lon = %u\n"
+                        "      SouthWest: lat = %u, lon = %u\n"
+                        "    Levels:     %u\n"
+                        "    Expiration: %u\n"
+                        "    ProductID:  %u\n"
+                        "    CRC:        %u\n"
+                        "    SigVersion: %u\n"
+                        "    SigOffset:  %u\n"
+                        "    ZOrder:     %u",
+                        JNXHeader.Version,
+                        JNXHeader.DeviceSN,
+                        JNXHeader.MapBounds.NorthEast.lat,
+                        JNXHeader.MapBounds.NorthEast.lon,
+                        JNXHeader.MapBounds.SouthWest.lat,
+                        JNXHeader.MapBounds.SouthWest.lon,
+                        JNXHeader.Levels,
+                        JNXHeader.Expiration,
+                        JNXHeader.ProductID,
+                        JNXHeader.CRC,
+                        JNXHeader.SigVersion,
+                        JNXHeader.SigOffset,
+                        JNXHeader.ZOrder);
+
+  if (JNXHeader.Levels > 20)
+    ThrowReaderException(CorruptImageError, ImproperImageHeader, image);
+
   /* Read JNX image level info. */
+  memset(JNXLevelInfo, 0, sizeof(JNXLevelInfo));
   total_tiles = 0;
   current_tile = 0;
   for (i = 0; i < JNXHeader.Levels; i++)
@@ -302,10 +337,22 @@
         {
           JNXLevelInfo[i].Copyright = NULL;
         }
-    }
+
+      if (EOFBlob(image))
+        ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
 
-  if (EOFBlob(image))
-    ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
+      if (image->logging)
+        (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                              "Level[%u] Info:"
+                              "  TileCount: %4u"
+                              "  TilesOffset: %6u"
+                              "  Scale: %04u",
+                              i,
+                              JNXLevelInfo[i].TileCount,
+                              JNXLevelInfo[i].TilesOffset,
+                              JNXLevelInfo[i].Scale
+                              );
+    }
 
   /* Get the current limit */
   SaveLimit = GetMagickResourceLimit(MapResource);
@@ -316,11 +363,32 @@
   /* Read JNX image data. */
   for (i = 0; i < JNXHeader.Levels; i++)
     {
+      /*
+        Validate TileCount against remaining file data
+      */
+      const magick_off_t current_offset = TellBlob(image);
+      const size_t pos_list_entry_size =
+        sizeof(magick_uint32_t) + sizeof(magick_uint32_t) + sizeof(magick_uint32_t) +
+        sizeof(magick_uint32_t) + sizeof(magick_uint16_t) + sizeof(magick_uint16_t) +
+        sizeof(magick_uint32_t) + sizeof(magick_uint32_t);
+      const magick_off_t remaining = file_size-current_offset;
+      const size_t needed = MagickArraySize(pos_list_entry_size,JNXLevelInfo[i].TileCount);
+
+      if ((needed == 0U) || (remaining <= 0) || (remaining < (magick_off_t) needed))
+        {
+          (void) SetMagickResourceLimit(MapResource, SaveLimit);
+          ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
+        }
+
       PositionList = MagickAllocateArray(TJNXTileInfo *,
                                          JNXLevelInfo[i].TileCount,
                                          sizeof(TJNXTileInfo));
       if (PositionList == NULL)
-        continue;
+        {
+          (void) SetMagickResourceLimit(MapResource, SaveLimit);
+          ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,
+                               image);
+        }
 
       (void) SeekBlob(image, JNXLevelInfo[i].TilesOffset, SEEK_SET);
       for (j = 0; j < JNXLevelInfo[i].TileCount; j++)
@@ -333,12 +401,15 @@
           PositionList[j].PicHeight = ReadBlobLSBShort(image);
           PositionList[j].PicSize = ReadBlobLSBLong(image);
           PositionList[j].PicOffset = ReadBlobLSBLong(image);
-        }
 
-      if (EOFBlob(image))
-        {
-          MagickFreeMemory(PositionList);
-          ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
+          if (EOFBlob(image) ||
+              ((magick_off_t) PositionList[j].PicOffset +
+               PositionList[j].PicSize > file_size))
+            {
+              (void) SetMagickResourceLimit(MapResource, SaveLimit);
+              MagickFreeMemory(PositionList);
+              ThrowReaderException(CorruptImageError,UnexpectedEndOfFile,image);
+            }
         }
 
       for (j = 0; j < JNXLevelInfo[i].TileCount; j++)
@@ -351,6 +422,9 @@
           image = ExtractTileJPG(image, image_info, PositionList+j, exception);
           (void) SetMonitorHandler(previous_handler);
 
+          if (exception->severity >= ErrorException)
+            break;
+
           current_tile++;
           if (QuantumTick(current_tile,total_tiles))
             if (!MagickMonitorFormatted(current_tile,total_tiles,exception,
--- a/magick/version.h	Tue Aug 22 08:08:30 2017 -0500
+++ b/magick/version.h	Sat Aug 26 14:14:13 2017 -0500
@@ -38,8 +38,8 @@
 #define MagickLibVersion  0x191600
 #define MagickLibVersionText  "1.4"
 #define MagickLibVersionNumber 19,16,0
-#define MagickChangeDate   "20170814"
-#define MagickReleaseDate  "snapshot-20170814"
+#define MagickChangeDate   "20170826"
+#define MagickReleaseDate  "snapshot-20170826"
 	
 /*
   The MagickLibInterfaceNewest and MagickLibInterfaceOldest defines
--- a/www/Changelog.html	Tue Aug 22 08:08:30 2017 -0500
+++ b/www/Changelog.html	Sat Aug 26 14:14:13 2017 -0500
@@ -35,6 +35,16 @@
 <div class="document">
 
 
+<p>2017-08-26  Bob Friesenhahn  &lt;<a class="reference external" href="mailto:bfriesen&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">bfriesen<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
+<blockquote>
+<ul class="simple">
+<li>coders/jnx.c (ReadJNXImage): Fix denial of service (DOS) issue
+in ReadJNXImage() whereby large amounts of CPU and memory
+resources may be consumed although the file itself does not
+support the requests.  Problem was reported via email on Wed Aug
+23 2017 by Xiaohei and Wangchu from Alibaba Security Team.</li>
+</ul>
+</blockquote>
 <p>2017-08-14  Glenn Randers-Pehrson  &lt;<a class="reference external" href="mailto:glennrp&#37;&#52;&#48;simple&#46;dallas&#46;tx&#46;us">glennrp<span>&#64;</span>simple<span>&#46;</span>dallas<span>&#46;</span>tx<span>&#46;</span>us</a>&gt;</p>
 <blockquote>
 <ul class="simple">