CVE-2016-4562 The DrawDashPolygon function in MagickCore/draw.c in ImageMagick before 6.9.4-0 and 7.x before 7.0.1-2 mishandles calculations of certain vertices integer 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.
CVE-2016-4563 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. CVE-2016-4564 The DrawImage function in MagickCore/draw.c in ImageMagick before 6.9.4-0 and 7.x before 7.0.1-2 makes an incorrect function call in attempting to locate the next token, 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. In security tracker, all of these link to the same commit: https://github.com/ImageMagick/ImageMagick/commit/726812fa2fa7ce16bcf58f6e115f65427a1c0950 Prevent buffer overflow in magick/draw.c This commit appears to have a number of changes that aren't related to these CVEs, that I can tell: * A number of "alpha=(Quantum) TransparentAlpha" changed to "alpha=(MagickRealType) TransparentAlpha" I believe thiese a defined as: typedef long double MagickRealType; typedef double Quantum; * "number_vertices" to "(ssize_t) number_vertices" in for loop - might be significant, but is the case already in wheezy version. * "GetNextToken(q,&q,extent,keyword);" --> "GetNextToken(q,&q,MagickPathExtent,keyword);" wheezy doens't have this parameter. Not sure if that is good or bad. * "weight=StringToUnsignedLong(token)" --> "weight=(ssize_t) StringToUnsignedLong(token)" * Added newline character * "weight=StringToUnsignedLong(option);" --> "weight=(ssize_t) StringToUnsignedLong(option);" Significant changes to TraceStrokePolygon function: @@ -6021,17 +6021,29 @@ } 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) Am wondering if maybe only this last part is required - it merges cleanly too. Although not really entirely sure how this one function can fit all CVEs. Possibly this patch only fixes CVE-2016-4563? Anyway, out of time now, just wanted to summarize the situation so I don't forget... -- Brian May <b...@debian.org>
