Re: imagemagick
Hi,
On Tue, 26 Jan 2016, Brian May wrote:
> Just wondered why imagemagick was marked in data/dla-needed.txt?
According to git blame, it was added by Thorsten. If you want a clear
answer, it's best to ask him the question directly.
That said he put the comment "NOTE: only minor issues without CVE"
and I'm not sure why it didn't mark it as no-dsa given this description.
> It looks like due to the issue here:
> https://security-tracker.debian.org/tracker/TEMP-0811308-B63DA1
>
> Which is Debian bug: 811308:
>
> - Memory Leak while handle psd file
> http://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=28791
>
> - IM 6.9.2 crash with some PNG
> http://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=28466
>
> - Prevent null pointer access in magick/constitute.c
> https://github.com/ImageMagick/ImageMagick/pull/34
>
> - PixelColor off by one on i386
> https://github.com/ImageMagick/ImageMagick/issues/54
>
> - Fixed memory leak when reading incorrect PSD files
>
> For the memory leaks and null pointer issues: Do we take the pessimestic
> point of view and assume that they are security issues that need fixing,
> or should we be conservative?
Depending on how it's used, both issues can lead to denial of service...
> 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.
>
> Also, at what point do we decide that a CVE is needed for issues like
> this?
As soon as an issue is known to be a security vulnerability.
On Wed, 27 Jan 2016, Brian May wrote:
> Brian May <brian@linuxpenguins.xyz> writes:
> 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.
I would not make this assertion without double checking with the package
maintainer.
The most recent patches are more likely to be present in sid and in
squeeze because there has been no newer upstream releases... but the
two codebases have evolved and I would not expect all sid patches to be
relevant for squeeze.
> (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)
Or maybe they are already applied but under another name?
> 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.
The condition is present in both case, it seems to have just been moved
up before the GetCacheViewVirtualPixels call.
> 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.
Definitely not an option.
If anything I would stick with the problems reported and would not try to
find more possible issues to fix.
> 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.
The latest upstream release is in experimental, not in unstable. That
mighgt explain why you are not seeing patches disappear in sid... if you
want to make a judgment call about the supportability of imagemagick then
you would rather have to invest more time into analyzing the situation.
The codebase is certainly far from perfect but this tool is heavily used
and upstream developers are reasonably responsive so I don't think we
should consider dropping support.
Cheers,
--
Raphaël Hertzog ◈ Debian Developer
Support Debian LTS: http://www.freexian.com/services/debian-lts.html
Learn to master Debian: http://debian-handbook.info/get/
Reply to: