[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: imagemagick



Brian May <brian@linuxpenguins.xyz> writes:

> Possibly the 2nd issue is most likely a security issue (?):
> http://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=28466
> Fixed in:
> http://sources.debian.net/patches/patch/imagemagick/8:6.8.9.9-7/0072-Fixed-out-of-bounds-error-in-SpliceImage.patch/
> If I am reading this correctly, might be possible to trick imagemagick
> to read/write past the end of the buffer when splicing an image. Suspect
> exploiting this might be difficult. You would need to trick somebody (or
> maybe a web service) to run imagemagick with a splice operation possibly
> with a PNG image you supply, and I am not sure if you could do any more
> then cause a crash.

I can't help notice that this package has a huge number of Debian
patches to fix various crashes. 58 in squeeze and 75 in sid. At least
some of the squeeze patches are still present in the sid
version. Probably could make that many patches still present, although I
haven't checked.

e.g. the last patch in squeeze:
debian/patches/0058-Avoid-a-memory-leak-in-quantum-management.patch

Looks very similar to the patch in sid:
debian/patches/0047-Avoid-a-memory-leak-in-quantum-management.patch

As a result, I am inclined to think that all patches 0048 to 0075 from
sid might be equally relevant to the squeeze version.

0048-Avoid-a-crash-in-png-coder.patch
0049-Avoid-heap-overflow.patch
0050-Thread-limit-should-be-at-least-1.patch
0051-Fixed-parsing-resource-block.patch
0052-Fixed-usage-of-object-after-it-has-been-destroyed-an.patch
0053-Create-IsValidColormapIndex-function.patch
0054-Replaced-calls-to-ConstrainColormapIndex-with-IsVali.patch
0055-During-identification-of-image-do-not-fill-memory.patch
0056-Fix-correctly-the-xpm-crash-problem.patch
0057-Fix-a-miff-security-bug.patch
0058-Fix-a-DOS-in-viccar-file-handling.patch
0059-Fix-a-DOS-in-HDR-file.patch
0060-Fix-a-DOS-in-PDB-file-handling.patch
0061-Avoid-using-NULL-alpha_image-in-the-JNG-decoder.patch
0062-Jpeg-images-no-longer-have-pixels-per-inch-as-a-defa.patch
0063-Add-an-additional-check-for-end-of-file-for-the-RLE-.patch
0064-Fix-a-double-free-in-tga-file.patch
0065-Add-field-width-to-sscanf-to-prevent-buffer-overflow.patch
0066-Limit-fx-recursive-to-avoid-stack-overflow.patch
0067-Added-extra-checks-to-avoid-out-of-bounds-error-when.patch
0068-Fixed-size-of-memory-allocation-to-avoid-segfault-Gi.patch
0069-Fixed-memory-leak-when-reading-incorrect-PSD-files.patch
0070-Fix-PixelColor-off-by-one-on-i386.patch
0071-Prevent-null-pointer-access-in-magick-constitute.c.patch
0072-Fixed-out-of-bounds-error-in-SpliceImage.patch
0073-Fixed-memory-leaks.patch
0074-Fix-overflow-in-pict-image-parsing.patch
0075-Fix-buffer-overflow-in-icon-parsing-code.patch

(curiously some of these patches when applied are show up as reversed
patches when applied to the squeeze version; possibly an indication that
the squeeze version was fine and the solution was to revert back to that
code)


I tried applying the 0072 patch to the squeeze version; however it
doesn't apply cleanly. It almost applies with wiggle, however there is
one part that fails to apply that looks dodgy.

<<<<<<< found
    p=GetCacheViewVirtualPixels(image_view,0,y-splice_geometry.height,
      image->columns,1,exception);
||||||| expected
    p=GetCacheViewVirtualPixels(image_view,0,y-(ssize_t) splice_geometry.height,
      image->columns,1,exception);
    if ((y < 0) || (y >= (ssize_t) splice_image->rows))
      continue;
=======
    if ((y < 0) || (y >= (ssize_t)splice_image->rows))
      continue;
    p=GetCacheViewVirtualPixels(image_view,0,y-(ssize_t) splice_geometry.height,
      splice_image->columns,1,exception);
>>>>>>> replacement

i.e. I suspect that if condition not present in the squeeze version but
required by the patch in the sid version might actually be important in
preventing the problem the patch is suppose to fix. Could be in another
of the many patches.

It looks like none of the other patches shown above apply cleanly
either.

Another option would be to backport the sid version to squeeze, however
it looks like there has been considerable changes to the libraries that
would break code linked to the old libraries. So probably not a good
option.

I suspect everything I have said here will apply equally to the version
in wheezy.

Is this software we actually want to support? It looks like there are
heaps of bugs in the upstream code, and the fixes for some reason aren't
getting into new upstream releases.

I am inclined to say the risk of breaking something (or maybe fixing
broken behaviour that people rely on) plus the fact these are minor
vulnerabilities (if they are vulnerabilities at all) means we probably
shouldn't touch them.
-- 
Brian May <bam@debian.org>


Reply to: