segfault in png to gif conversion

[Dariusz Gadomski]

Regression between 8:6.8.9.9-7ubuntu5.9 and 8:6.8.9.9-7ubuntu5.12.

Test case:
1. Download the attached pngs.
2. Run:
/usr/bin/convert -limit memory 512MiB -limit map 0MiB -limit file 10 -delay 16 -loop 0 -coalesce -deconstruct ./*.png ./output.gif

Expected result:
Process finishes with resulting output.gif.

Actual result:
Process is aborted with SIGSEGV:

Other information:
In my tests looks like it has been introduced in 8:6.8.9.9-7ubuntu5.11 and does not occur on Bionic.

Stack trace:
#0 EncodeImage (image_info=0x645c40, data_size=<optimized out>,
    image=0x636890) at ../../coders/gif.c:676
#1 WriteGIFImage (image_info=0x640700, image=0x636890)
    at ../../coders/gif.c:1905
#2 0x00007ffff79a5f0f in WriteImage (image_info=image_info@entry=0x618680,
    image=image@entry=0x62cb30) at ../../magick/constitute.c:1184
#3 0x00007ffff79a684f in WriteImages (image_info=image_info@entry=0x60fcd0,
    images=<optimized out>, images@entry=0x62cb30, filename=<optimized out>,
    exception=exception@entry=0x602ea0) at ../../magick/constitute.c:1335
#4 0x00007ffff763e84e in ConvertImageCommand (image_info=0x60fcd0, argc=19,
    argv=0x6143b0, metadata=0x0, exception=0x602ea0)
    at ../../wand/convert.c:3215
#5 0x00007ffff76ab527 in MagickCommandGenesis (
    image_info=image_info@entry=0x60aab0,
    command=0x4007f0 <ConvertImageCommand@plt>, argc=argc@entry=19,
    argv=argv@entry=0x7fffffffdc68, metadata=metadata@entry=0x0,
    exception=exception@entry=0x602ea0) at ../../wand/mogrify.c:168
#6 0x0000000000400877 in ConvertMain (argv=0x7fffffffdc68, argc=19)
    at ../../utilities/convert.c:81
#7 main (argc=19, argv=0x7fffffffdc68) at ../../utilities/convert.c:92

[Marc Deslauriers (mdeslaur)]

Thanks for reporting this issue. Looks like we're possibly missing a couple of commits:

https://github.com/ImageMagick/ImageMagick6/commit/e5e87c087ed48db886be0ff3aff4041d38218192
https://github.com/ImageMagick/ImageMagick6/commit/f5d04fc678f67984a1f8c1008dc8eac8ee7e3629

I'll prepare a regression fix.

[Marc Deslauriers (mdeslaur)]

The two additional commits don't seem to fix the segfault. I'm leaning towards reverting 0261-CVE-2017-13144.patch for now...

[Steve Beattie (sbeattie)]

Thanks Marc. I'm preparing an additional imagemagick update, so I'll incorporate that into it. Assigning to myself.

[Launchpad Janitor (janitor)]

This bug was fixed in the package imagemagick - 8:6.7.7.10-6ubuntu3.13

---------------
imagemagick (8:6.7.7.10-6ubuntu3.13) trusty-security; urgency=medium

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution vulnerabilities in ghostscript as
    invoked by imagemagick
    - debian/patches/200-disable-ghostscript-formats.patch: disable
      ghostscript handled types by default in policy.xml
  * SECURITY UPDATE: information leak in ReadXBMImage
    - debian/patches/CVE-2018-16323.patch: don't leave data
      uninitialized with negative pixels
    - CVE-2018-16323
  * SECURITY UPDATE: memory leak of colormap in WriteMPCImage
    - debian/patches/CVE-2018-14434.patch: free colormap on bad
      color depth
    - CVE-2018-14434
  * SECURITY UPDATE: memory leak in DecodeImage
    - debian/patches/CVE-2018-14435.patch: free memory when given a
      bad plane
    - CVE-2018-14435
  * SECURITY UPDATE: memory leak in ReadMIFFImage
    - debian/patches/CVE-2018-14436.patch: free memory when given a bad
      depth
    - CVE-2018-14436
  * SECURITY UPDATE: memory leak in parse8BIM
    - debian/patches/CVE-2018-14437-prereq.patch: check for negative
      values
    - debian/patches/CVE-2018-14437.patch: free strings in error
      conditions
    - CVE-2018-14437
  * SECURITY UPDATE: memory leak in ReadOneJNGImage
    - debian/patches/CVE-2018-16640-prereq-1.patch: define DestroyJNG()
    - debian/patches/CVE-2018-16640-prereq-2.patch: fix DestroyJNG()
    - debian/patches/CVE-2018-16640.patch: free memory on error
    - CVE-2018-16640
  * SECURITY UPDATE: denial of service due to out-of-bounds write
    in InsertRow
    - debian/patches/CVE-2018-16642.patch: improve checking for errors
    - CVE-2018-16642
  * SECURITY UPDATE: denial of service due to missing fputc checks
    - debian/patches/CVE-2018-16643.patch: check fputc calls for error
    - CVE-2018-16643
  * SECURITY UPDATE: denial of service in ReadDCMImage and
    ReadPICTImage
    - debian/patches/CVE-2018-16644-prereq-1.patch: make
      ReadRectangle() a boolean returning function and use it.
    - debian/patches/CVE-2018-16644-prereq-2.patch: check for EOF
      when reading from file
    - debian/patches/CVE-2018-16644-prereq-3.patch: define
      ThrowPICTException() macro and use it
    - debian/patches/CVE-2018-16644-1.patch,
      debian/patches/CVE-2018-16644-2.patch: check for invalid length
    - CVE-2018-16644
  * SECURITY UPDATE: excessive memory allocation issue in ReadBMPImage
    - debian/patches/CVE-2018-16645.patch: ensure number_colors is
      not too large
    - CVE-2018-16645
  * SECURITY UPDATE: denial of service in ReadOneJNGImage
    - debian/patches/CVE-2018-16749.patch; check for NULL color_image
    - CVE-2018-16749
  * SECURITY UPDATE: memory leak in formatIPTCfromBuffer
    - debian/patches/CVE-2018-16750.patch: free memory on error
    - CVE-2018-16750

  [ Marc Deslauriers ]
  * SECURITY REGRESSION: segfault in png to gif conversion (LP: #1793485)
    - debian/patches/0297-CVE-2017-13144.patch: removed pending further
      investigation.
    - debian/patches/CVE-2017-12430.patch: refreshed.

 -- Steve Beattie <email address hidden> Fri, 28 Sep 2018 11:...

[Launchpad Janitor (janitor)]

This bug was fixed in the package imagemagick - 8:6.8.9.9-7ubuntu5.13

---------------
imagemagick (8:6.8.9.9-7ubuntu5.13) xenial-security; urgency=medium

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution vulnerabilities in ghostscript as
    invoked by imagemagick
    - debian/patches/200-disable-ghostscript-formats.patch: disable
      ghostscript handled types by default in policy.xml
  * SECURITY UPDATE: information leak in ReadXBMImage
    - debian/patches/CVE-2018-16323.patch: don't leave data
      uninitialized with negative pixels
    - CVE-2018-16323
  * SECURITY UPDATE: memory leak of colormap in WriteMPCImage
    - debian/patches/CVE-2018-14434.patch: free colormap on bad
      color depth
    - CVE-2018-14434
  * SECURITY UPDATE: memory leak in DecodeImage
    - debian/patches/CVE-2018-14435.patch: free memory when given a
      bad plane
    - CVE-2018-14435
  * SECURITY UPDATE: memory leak in ReadMIFFImage
    - debian/patches/CVE-2018-14436.patch: free memory when given a
      bad depth
    - CVE-2018-14436
  * SECURITY UPDATE: memory leak in parse8BIM
    - debian/patches/CVE-2018-14437-prereq.patch: check for negative
      values
    - debian/patches/CVE-2018-14437.patch: free strings in error
      conditions
    - CVE-2018-14437
  * SECURITY UPDATE: memory leak in ReadOneJNGImage
    - debian/patches/CVE-2018-16640-prereq-1.patch: define DestroyJNG()
    - debian/patches/CVE-2018-16640-prereq-2.patch: fix DestroyJNG()
    - debian/patches/CVE-2018-16640.patch: free memory on error
    - CVE-2018-16640
  * SECURITY UPDATE: denial of service due to out-of-bounds write
    in InsertRow
    - debian/patches/CVE-2018-16642.patch: improve checking for errors
    - CVE-2018-16642
  * SECURITY UPDATE: denial of service due to missing fputc checks
    - debian/patches/CVE-2018-16643.patch: check fputc calls for error
    - CVE-2018-16643
  * SECURITY UPDATE: denial of service in ReadDCMImage and
    ReadPICTImage
    - debian/patches/CVE-2018-16644-prereq-1.patch: check for EOF
      when reading from file
    - debian/patches/CVE-2018-16644-prereq-2.patch: define
      ThrowPICTException() macro and use it
    - debian/patches/CVE-2018-16644-1.patch,
      debian/patches/CVE-2018-16644-2.patch: check for invalid length
    - CVE-2018-16644
  * SECURITY UPDATE: excessive memory allocation issue in ReadBMPImage
    - debian/patches/CVE-2018-16645.patch: ensure number_colors is
      not too large
    - CVE-2018-16645
  * SECURITY UPDATE: denial of service in ReadOneJNGImage
    - debian/patches/CVE-2018-16749.patch; check for NULL color_image
    - CVE-2018-16749
  * SECURITY UPDATE: memory leak in formatIPTCfromBuffer
    - debian/patches/CVE-2018-16750.patch: free memory on error
    - CVE-2018-16750

  [ Marc Deslauriers ]
  * SECURITY REGRESSION: segfault in png to gif conversion (LP: #1793485)
    - debian/patches/0261-CVE-2017-13144.patch: removed pending
      further investigation.
    - debian/patches/CVE-2017-12430.patch: refreshed.

 -- Steve Beattie <email address hidden> Fri, 28 Sep 2018 11:19:54 -0700

[Hajo Locke (hajo-locke)]

Hello,

can last changelogentry assigned to one particular CVE Number?

* SECURITY UPDATE: code execution vulnerabilities in ghostscript as
    invoked by imagemagick
    - debian/patches/200-disable-ghostscript-formats.patch: disable
      ghostscript handled types by default in policy.xml

We have reports of users who cant convert pdf-files any more, because policy.xml is forbidding it.
Which risk will we take if we change policy.xml back to former content?

And i think there is a typo in changelog. filename is 300-disable-ghostscript-formats.patch

[Seth Arnold (seth-arnold)]

Hello Hajo,

Tavis Ormandy has recently discovered enough flaws in ghostscript that the general consensus in the security community is that it is not safe to allow ghostscript to process untrusted inputs. See for example:

    I think we should encourage switching to other document
    formats that we have a better handle on securing. If you
    do need untrusted ps, I think treating it the same as
    shell script file you downloaded from the internet.

https://www.openwall.com/lists/oss-security/2018/10/09/6

ImageMagick is a well-known and widely-available attack vector.

Whoever would wish to use ImageMagick on untrusted inputs should prepare an AppArmor profile (or SELinux/SMACK/TOMOYO policy) to reflect their expected usage to restrict how much damage can be done, and modify the policy.xml file to explicitly allow using ghostscript through ImageMagick: https://imagemagick.org/script/security-policy.php

We debated if this was a change we wanted to make because we knew that it would inconvenience some of our users. However, we feel that someone who needs these tools should know the full risks of these tools and thus be able to mitigate the risks as appropriate in their own environment.

Thanks

[Hajo Locke (hajo-locke)]

Hello Seth,

thanks for your answer. My expectations was that flaws are fixed in code and it's not necessary to block filetypes. Unfortunately in hosting/webapplications/shops ImageMagick+Ghostscript are widely used in Standardsoftware. Minutes after Updating some Servers we had reports about failing conversions.
We use apparmor-profiles to protect our internal structure. User itself is only able to read some needed paths and write to his own home. So this risk should be ok for us unless there is an exploit to gain root privileges. If i understand Tavis Ormandy's statement the right way, more flaws are highly probable.

Thanks,
Hajo