CVE-2016-4563: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The TraceStrokePolygon function in MagickCore/draw.c in ImageMagick before 6.9.4-0 and 7.x before 7.0.1-2 mishandles the relationship between the BezierQuantum value and certain strokes data, which allows remote attackers to cause a denial of service (buffer overflow and application crash) or possibly have unspecified other impact via a crafted file.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 8.8
Published: 2016-06-04T16:59Z
Repository: https://github.com/ImageMagick/ImageMagick
Language: Other
Files changed: ChangeLog, MagickCore/draw.c
Commit date: 2016-05-04 19:09:35 -0400
Commit message: Prevent buffer overflow in magick/draw.c

Diff: diff --git a/ChangeLog b/ChangeLog
index 1e88dc94cc..d2dfe6830a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
+2016-05-04  7.0.1-2 Cristy  <quetzlzacatenango@image...>
+  * Check for buffer overflow in magick/draw.c/DrawStrokePolygon().
+
 2016-05-03  7.0.1-1 Cristy  <quetzlzacatenango@image...>
   * New version 7.0.1-1, GIT revision 10723:9fc8a0c:20160503.
 
 2016-05-03  7.0.1-1 Cristy  <quetzlzacatenango@image...>
   * Sanitize input filename for http / https delegates (improved patch).
diff --git a/MagickCore/draw.c b/MagickCore/draw.c
index 547678410b..d9fe9e7f2d 100644
--- a/MagickCore/draw.c
+++ b/MagickCore/draw.c
@@ -1415,11 +1415,11 @@ MagickExport MagickBooleanType DrawClipPath(Image *image,
   clip_mask=CloneImage(image,image->columns,image->rows,MagickTrue,exception);
   if (clip_mask == (Image *) NULL)
     return(MagickFalse);
   (void) QueryColorCompliance("#0000",AllCompliance,
     &clip_mask->background_color,exception);
-  clip_mask->background_color.alpha=(Quantum) TransparentAlpha;
+  clip_mask->background_color.alpha=(MagickRealType) TransparentAlpha;
   (void) SetImageBackgroundColor(clip_mask,exception);
   if (image->debug != MagickFalse)
     (void) LogMagickEvent(DrawEvent,GetMagickModule(),"\nbegin clip-path %s",
       draw_info->clip_mask);
   clone_info=CloneDrawInfo((ImageInfo *) NULL,draw_info);
@@ -1539,11 +1539,11 @@ static MagickBooleanType DrawDashPolygon(const DrawInfo *draw_info,
     n++;
   }
   status=MagickTrue;
   maximum_length=0.0;
   total_length=0.0;
-  for (i=1; (i < number_vertices) && (length >= 0.0); i++)
+  for (i=1; (i < (ssize_t) number_vertices) && (length >= 0.0); i++)
   {
     dx=primitive_info[i].point.x-primitive_info[i-1].point.x;
     dy=primitive_info[i].point.y-primitive_info[i-1].point.y;
     maximum_length=hypot((double) dx,dy);
     if (length == 0.0)
@@ -1792,11 +1792,11 @@ MagickExport MagickBooleanType DrawImage(Image *image,const DrawInfo *draw_info,
   for (q=primitive; *q != '\0'; )
   {
     /*
       Interpret graphic primitive.
     */
-    GetNextToken(q,&q,extent,keyword);
+    GetNextToken(q,&q,MagickPathExtent,keyword);
     if (*keyword == '\0')
       break;
     if (*keyword == '#')
       {
         /*
@@ -2102,11 +2102,11 @@ MagickExport MagickBooleanType DrawImage(Image *image,const DrawInfo *draw_info,
               weight;
 
             GetNextToken(q,&q,extent,token);
             weight=ParseCommandOption(MagickWeightOptions,MagickFalse,token);
             if (weight == -1)
-              weight=StringToUnsignedLong(token);
+              weight=(ssize_t) StringToUnsignedLong(token);
             graphic_context[n]->weight=(size_t) weight;
             break;
           }
         status=MagickFalse;
         break;
@@ -2351,11 +2351,12 @@ MagickExport MagickBooleanType DrawImage(Image *image,const DrawInfo *draw_info,
                   graphic_context[n]->affine.ty;
                 (void) FormatLocaleString(key,MagickPathExtent,"%s",name);
                 (void) SetImageArtifact(image,key,token);
                 (void) FormatLocaleString(key,MagickPathExtent,"%s-type",name);
                 (void) SetImageArtifact(image,key,type);
-                (void) FormatLocaleString(key,MagickPathExtent,"%s-geometry",name);
+                (void) FormatLocaleString(key,MagickPathExtent,"%s-geometry",
+                  name);
                 (void) FormatLocaleString(geometry,MagickPathExtent,
                   "%gx%g%+.15g%+.15g",
                   MagickMax(fabs(bounds.x2-bounds.x1+1.0),1.0),
                   MagickMax(fabs(bounds.y2-bounds.y1+1.0),1.0),
                   bounds.x1,bounds.y1);
@@ -4606,11 +4607,11 @@ MagickExport MagickBooleanType DrawPrimitive(Image *image,
           /*
             Draw dash polygon.
           */
           clone_info=CloneDrawInfo((ImageInfo *) NULL,draw_info);
           clone_info->stroke_width=0.0;
-          clone_info->stroke.alpha=(Quantum) TransparentAlpha;
+          clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
           status&=DrawPolygonPrimitive(image,clone_info,primitive_info,
             exception);
           clone_info=DestroyDrawInfo(clone_info);
           (void) DrawDashPolygon(draw_info,primitive_info,image,exception);
           break;
@@ -4641,11 +4642,11 @@ MagickExport MagickBooleanType DrawPrimitive(Image *image,
                 exception);
               break;
             }
           clone_info=CloneDrawInfo((ImageInfo *) NULL,draw_info);
           clone_info->stroke_width=0.0;
-          clone_info->stroke.alpha=(Quantum) TransparentAlpha;
+          clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
           status&=DrawPolygonPrimitive(image,clone_info,primitive_info,
             exception);
           clone_info=DestroyDrawInfo(clone_info);
           status&=DrawStrokePolygon(image,draw_info,primitive_info,exception);
           break;
@@ -4741,11 +4742,11 @@ static MagickBooleanType DrawStrokePolygon(Image *image,
   if (clone_info->fill_pattern != (Image *) NULL)
     clone_info->fill_pattern=DestroyImage(clone_info->fill_pattern);
   if (clone_info->stroke_pattern != (Image *) NULL)
     clone_info->fill_pattern=CloneImage(clone_info->stroke_pattern,0,0,
       MagickTrue,exception);
-  clone_info->stroke.alpha=(Quantum) TransparentAlpha;
+  clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
   clone_info->stroke_width=0.0;
   clone_info->fill_rule=NonZeroRule;
   status=MagickTrue;
   for (p=primitive_info; p->primitive != UndefinedPrimitive; p+=p->coordinates)
   {
@@ -4856,11 +4857,11 @@ MagickExport void GetDrawInfo(const ImageInfo *image_info,DrawInfo *draw_info)
   draw_info->linecap=ButtCap;
   draw_info->linejoin=MiterJoin;
   draw_info->miterlimit=10;
   draw_info->decorate=NoDecoration;
   draw_info->pointsize=12.0;
-  draw_info->undercolor.alpha=(Quantum) TransparentAlpha;
+  draw_info->undercolor.alpha=(MagickRealType) TransparentAlpha;
   draw_info->compose=OverCompositeOp;
   draw_info->render=MagickTrue;
   draw_info->debug=IsEventLogging();
   draw_info->stroke_antialias=clone_info->antialias;
   if (clone_info->font != (char *) NULL)
@@ -4923,11 +4924,11 @@ MagickExport void GetDrawInfo(const ImageInfo *image_info,DrawInfo *draw_info)
       ssize_t
         weight;
 
       weight=ParseCommandOption(MagickWeightOptions,MagickFalse,option);
       if (weight == -1)
-        weight=StringToUnsignedLong(option);
+        weight=(ssize_t) StringToUnsignedLong(option);
       draw_info->weight=(size_t) weight;
     }
   exception=DestroyExceptionInfo(exception);
   draw_info->signature=MagickCoreSignature;
   clone_info=DestroyImageInfo(clone_info);
@@ -6019,21 +6020,33 @@ static PrimitiveInfo *TraceStrokePolygon(const DrawInfo *draw_info,
           box_q[3].y)/(slope.p-slope.q));
         box_q[4].y=(double) (slope.p*(box_q[4].x-box_q[0].x)+box_q[0].y);
       }
     if (q >= (ssize_t) (max_strokes-6*BezierQuantum-360))
       {
-         max_strokes+=6*BezierQuantum+360;
-         path_p=(PointInfo *) ResizeQuantumMemory(path_p,(size_t) max_strokes,
-           sizeof(*path_p));
-         path_q=(PointInfo *) ResizeQuantumMemory(path_q,(size_t) max_strokes,
-           sizeof(*path_q));
-         if ((path_p == (PointInfo *) NULL) || (path_q == (PointInfo *) NULL))
-           {
-             polygon_primitive=(PrimitiveInfo *)
-               RelinquishMagickMemory(polygon_primitive);
-             return((PrimitiveInfo *) NULL);
-           }
+        if (~max_strokes < (6*BezierQuantum+360))
+          {
+            path_p=(PointInfo *) RelinquishMagickMemory(path_p);
+            path_q=(PointInfo *) RelinquishMagickMemory(path_q);
+          }
+        else
+          {
+            max_strokes+=6*BezierQuantum+360;
+            path_p=(PointInfo *) ResizeQuantumMemory(path_p,max_strokes,
+              sizeof(*path_p));
+            path_q=(PointInfo *) ResizeQuantumMemory(path_q,max_strokes,
+              sizeof(*path_q));
+          }
+        if ((path_p == (PointInfo *) NULL) || (path_q == (PointInfo *) NULL))
+          {
+            if (path_p != (PointInfo *) NULL)
+              path_p=(PointInfo *) RelinquishMagickMemory(path_p);
+            if (path_q != (PointInfo *) NULL)
+              path_q=(PointInfo *) RelinquishMagickMemory(path_q);
+            polygon_primitive=(PrimitiveInfo *)
+              RelinquishMagickMemory(polygon_primitive);
+            return((PrimitiveInfo *) NULL);
+          }
       }
     dot_product=dx.q*dy.p-dx.p*dy.q;
     if (dot_product <= 0.0)
       switch (draw_info->linejoin)
       {

Vulnerable code:   clip_mask->background_color.alpha=(Quantum) TransparentAlpha;
  for (i=1; (i < number_vertices) && (length >= 0.0); i++)
    GetNextToken(q,&q,extent,keyword);
              weight=StringToUnsignedLong(token);
                (void) FormatLocaleString(key,MagickPathExtent,"%s-geometry",name);
          clone_info->stroke.alpha=(Quantum) TransparentAlpha;
          clone_info->stroke.alpha=(Quantum) TransparentAlpha;
  clone_info->stroke.alpha=(Quantum) TransparentAlpha;
  draw_info->undercolor.alpha=(Quantum) TransparentAlpha;
        weight=StringToUnsignedLong(option);
         max_strokes+=6*BezierQuantum+360;
         path_p=(PointInfo *) ResizeQuantumMemory(path_p,(size_t) max_strokes,
           sizeof(*path_p));
         path_q=(PointInfo *) ResizeQuantumMemory(path_q,(size_t) max_strokes,
           sizeof(*path_q));
         if ((path_p == (PointInfo *) NULL) || (path_q == (PointInfo *) NULL))
           {
             polygon_primitive=(PrimitiveInfo *)
               RelinquishMagickMemory(polygon_primitive);
             return((PrimitiveInfo *) NULL);
           }
Fixed code: 2016-05-04  7.0.1-2 Cristy  <quetzlzacatenango@image...>
  * Check for buffer overflow in magick/draw.c/DrawStrokePolygon().

  clip_mask->background_color.alpha=(MagickRealType) TransparentAlpha;
  for (i=1; (i < (ssize_t) number_vertices) && (length >= 0.0); i++)
    GetNextToken(q,&q,MagickPathExtent,keyword);
              weight=(ssize_t) StringToUnsignedLong(token);
                (void) FormatLocaleString(key,MagickPathExtent,"%s-geometry",
                  name);
          clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
          clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
  clone_info->stroke.alpha=(MagickRealType) TransparentAlpha;
  draw_info->undercolor.alpha=(MagickRealType) TransparentAlpha;
        weight=(ssize_t) StringToUnsignedLong(option);
        if (~max_strokes < (6*BezierQuantum+360))
          {
            path_p=(PointInfo *) RelinquishMagickMemory(path_p);
            path_q=(PointInfo *) RelinquishMagickMemory(path_q);
          }
        else
          {
            max_strokes+=6*BezierQuantum+360;
            path_p=(PointInfo *) ResizeQuantumMemory(path_p,max_strokes,
              sizeof(*path_p));
            path_q=(PointInfo *) ResizeQuantumMemory(path_q,max_strokes,
              sizeof(*path_q));
          }
        if ((path_p == (PointInfo *) NULL) || (path_q == (PointInfo *) NULL))
          {
            if (path_p != (PointInfo *) NULL)
              path_p=(PointInfo *) RelinquishMagickMemory(path_p);
            if (path_q != (PointInfo *) NULL)
              path_q=(PointInfo *) RelinquishMagickMemory(path_q);
            polygon_primitive=(PrimitiveInfo *)
              RelinquishMagickMemory(polygon_primitive);
            return((PrimitiveInfo *) NULL);
          }