#345 MagickTrimImage with extreme fuzz can produce image with negative width

[Don]

Performing a MagickTrimImage that completely strips the image away, results in a negative width.
Eg for a 500 pixel-wide image, the resulting width is -500.
I would expect width to be in the range 0..500.

Since width is unsigned, this behaves as a 4 billion x 4 billion pixel image dimensions. Bad things happen after that.

I know this is an old version, the bug may already have been fixed -- but I haven't found any reference to it.

---

// Using a 16-bit build of GraphicsMagick
// GraphicsMagick 1.3.19 2013-12-31 Q16 http://www.GraphicsMagick.org/

// img_ptr, img_length are pointing to a white JPEG blob, 500 x 500 pixels.

InitializeMagick(null);

MagicWand *magick_wand = NewMagickWand();

bool ok = MagickReadImageBlob(magick_wand, img_ptr, img_length);

   // Solid white

    PixelWand *pixel_wand = NewPixelWand();

    PixelSetOpacity(pixel_wand, 1.0); // completely opaque
    PixelSetRedQuantum(pixel_wand, 65535);
    PixelSetGreenQuantum(pixel_wand, 65535);
    PixelSetBlueQuantum(pixel_wand, 65535);

    MagickSetImageBackgroundColor(magick_wand, pixel_wand);

    double fuzz = 3.0 * (256.0 * 256.0) / 100.0;

    MagickTrimImage(magick_wand, fuzz);

    long wid = MagickGetImageWidth(magick_wand);

// wid is now less than 0.

[Bob Friesenhahn]

I assume that there is some problem in magick/transform.c around line 508 (insufficient validation) or perhaps in GetImageBoundingBox() itself (production of bad values). I don't see any related fixes subsequent to 1.3.19.

Your own code is contributing to the problem since MagickGetImageWidth() returns an unsigned type and you are assigning it to a signed type.

[Bob Friesenhahn]

A great many integer overflows and sign-extension problems were fixed while resolving Coverity issues. Note was made of these in the ChangeLog file but often it would require being able to see Coverity's diagnosis (with annotated source code) in order to understand the ChangeLog entries.

Are you able to reproduce this with the current release? Are you able to reproduce it via the GraphicsMagick command-line using the version you have?

For example does this simple test produce unexpected width values with the version you are using?

~~~~~~~~~~~~~~~~~~~~~~
% gm convert -verbose -size 500x500 xc:white -fuzz 1966.08 -trim null:
white XC 500x500+0+0 PseudoClass 1c 8-bit 0.000u 0:01
white XC 500x500+0+0 PseudoClass 1c 8-bit 0.020u 0:01
white=> NULL 500x500+0+0 PseudoClass 1c 8-bit 0.000u 0:01
~~~~~~~~~~~~~~~~~~~~~~~

Bob

[Don]

I cannot reproduce the bug using the command-line test that you've described, in either 8 bit or 16 bit builds of 1.3.19. It only happens when using the lib.

[Don]

For reference, here is a sample image which produces the error (just in case some more subtle feature of the image is required to trigger the condition)

http://www.moebel.de/xyz/9870128644126/500x500/Polsterstoff-Moebelstoff-Molto-Polyacryl-Teflon/Polsterstoff-Moebelstoff-Molto-Polyacryl-Teflon.jpg

[Troy Patteson]

I just had a quick look at this in the debugger and the issue appears to be in GetImageBoundingBox() in magick/analyze.c. The bounds structure starts with values:

bounds = {width = 0, height = 0, x = 500, y = 500}

I think width and height of zero indicates that no boundary has been found. At some point it gets to these values:

{width = 0, height = 498, x = 496, y = 498}

and runs this:

if ((bounds.width != 0) || (bounds.height != 0))
{
bounds.width-=(bounds.x-1);
bounds.height-=(bounds.y-1);
}

where width becomes negative.

I suspect the width and height should be tested separately as:

if (bounds.width != 0)
{
bounds.width-=(bounds.x-1);
}
if (bounds.height != 0)
{
bounds.height-=(bounds.y-1);
}

When I ran it with that change I got back the original image as no bounding box was found. I then ran it on an image with a border and got back the image with the border trimmed.

Troy

[Bob Friesenhahn]

This problem has been fixed using the solution suggested by Troy Patteson. The fix is provided by Mercurial changeset 16336:1a4bc46dca46. Thanks for the report!