Fwd: Re: BUG: graphicsmagick CVE-2016-5240 wrong in Debian-Wheezy
[Forwarding after getting ACK]
----- Original message -----
From: Chris Lamb <lamby@debian.org>
To: Philipp Hahn <hahn@univention.de>, security@debian.org, "Laszlo Boszormenyi (GCS)" <gcs@debian.org>
Cc: Bob Friesenhahn <bfriesen@GraphicsMagick.org>
Subject: Re: BUG: graphicsmagick CVE-2016-5240 wrong in Debian-Wheezy
Date: Tue, 13 Dec 2016 17:34:20 +0100
Philipp Hahn wrote:
> Hello Chris,
>
> @Bob: it's a Debian only bug, just FYI
>
> the issue <https://security-tracker.debian.org/tracker/CVE-2016-5240> is
> not fixed correctly in Debian-Wheezy; it is broken since first
> 1.3.16-1.1+deb7u3 and still in latest 1.3.16-1.1+deb7u5: "gm" crashes
> with a SEGV using the original test data from
> <http://seclists.org/oss-sec/2016/q2/180>:
Oh dear. I've marked this as such in the trackers and it should be worked
on ASAP. Note that the DLA announcement mail also included a typo:
https://lists.debian.org/debian-lts-announce/2016/07/msg00008.html
ie. DLA-574-1 should be DLA-547-1.
Philip? Can I forward this mail to the debian-lts mailing list? (Quoting
in full below so I sent it verbatim if you ACK…)
> wget -O circular.svg http://seclists.org/oss-sec/2016/q2/att-180
> /circular_svg.bin
> gdb --args gm convert circular.svg bmp:/dev/null
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff79c88d8 in DrawImage (image=image@entry=0x55555576d490, draw_info=draw_info@entry=0x555555769320)
> > at magick/render.c:2518
> > 2518 graphic_context[n]->dash_pattern[j-x];
> > (gdb) print graphic_context[n]->dash_pattern
> > $4 = (double *) 0x0
> > (gdb) bt
> > #0 0x00007ffff79c88d8 in DrawImage (image=image@entry=0x55555576d490, draw_info=draw_info@entry=0x555555769320)
> > at magick/render.c:2518
> > #1 0x00007ffff7a662c4 in ReadMVGImage (image_info=0x555555766f00, exception=0x7fffffffde00) at coders/mvg.c:186
> > #2 0x00007ffff795f7e8 in ReadImage (image_info=image_info@entry=0x55555576a900,
> > exception=exception@entry=0x7fffffffde00) at magick/constitute.c:1596
> > #3 0x00007ffff7a97826 in ReadSVGImage (image_info=0x555555764db0, exception=0x7fffffffde00) at coders/svg.c:2752
> > #4 0x00007ffff795f7e8 in ReadImage (image_info=image_info@entry=0x555555760be0,
> > exception=exception@entry=0x7fffffffde00) at magick/constitute.c:1596
> > #5 0x00007ffff794b754 in ConvertImageCommand (image_info=0x555555760be0, argc=3, argv=0x555555762d30,
> > metadata=0x0, exception=0x7fffffffde00) at magick/command.c:3998
> > #6 0x00007ffff79329b3 in MagickCommand (image_info=image_info@entry=0x555555760be0, argc=argc@entry=3,
> > argv=argv@entry=0x7fffffffe750, metadata=metadata@entry=0x7fffffffddf8,
> > exception=exception@entry=0x7fffffffde00) at magick/command.c:8314
> > #7 0x00007ffff7957e0d in GMCommand (argc=3, argv=0x7fffffffe750) at magick/command.c:16252
> > #8 0x00007ffff4799ead in __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
> > #9 0x00005555555548e1 in _start ()
>
> The version in Jessie is not affected.
>
> magick/render.c:
> > 2493 graphic_context[n]->dash_pattern=
> > 2494 MagickAllocateArray(double *,(2*x+1),sizeof(double));
> here the memory is allocated
> > 2495 if (graphic_context[n]->dash_pattern == (double *) NULL)
> > 2496 {
> > 2497 ThrowException3(&image->exception,ResourceLimitError,
> > 2498 MemoryAllocationFailed,UnableToDrawOnImage);
> > 2499 break;
> > 2500 }
> > 2501 for (j=0; j < x; j++)
> > 2502 {
> > 2503 MagickGetToken(q,&q,token,token_max_length);
> > 2504 if (*token == ',')
> > 2505 MagickGetToken(q,&q,token,token_max_length);
> > 2506 graphic_context[n]->dash_pattern[j]=MagickAtoF(token);
> > 2507 if (graphic_context[n]->dash_pattern[j] < 0.0)
> > 2508 status=MagickFail;
> > 2509 if (status == MagickFail)
> > 2510 {
> > 2511 MagickFreeMemory(graphic_context[n]->dash_pattern);
> here it is freed
> > 2512 break;
> this only breaks the loop from line 2501, but not the switch/case statement.
> > 2513 }
> > 2514 }
> WRONG! This is too late and needs to go up 6 lines.
> > 2515 if (x & 0x01)
> > 2516 for ( ; j < (2*x); j++)
> > 2517 graphic_context[n]->dash_pattern[j]=
> > 2518 graphic_context[n]->dash_pattern[j-x];
> here it crashes
>
>
> The Debian patch
> graphicsmagick-1.3.16/debian/patches/CVE-2016-5240.patch is buggy:
> > 31 @@ -2505,6 +2504,13 @@
> > 32 if (*token == ',')
> > 33 MagickGetToken(q,&q,token,token_max_length);
> > 34 graphic_context[n]->dash_pattern[j]=MagickAtoF(token);
> > 35 + if (graphic_context[n]->dash_pattern[j] < 0.0)
> > 36 + status=MagickFail;
> > 37 + if (status == MagickFail)
> > 38 + {
> > 39 + MagickFreeMemory(graphic_context[n]->dash_pattern);
> > 40 + break;
> > 41 + }
> > 42 }
> ^^^^^^^^^^^^^^^^^^^^^^^^
> > 43 if (x & 0x01)
> > 44 for ( ; j < (2*x); j++)
>
> And here the original patch from
> <https://sourceforge.net/p/graphicsmagick/code/ci/ddc999ec896ce8ac372a91443d6b9e4826f75b52/#diff-2>:
> > @@ -2570,7 +2569,14 @@ DrawImage(Image *image,const DrawInfo *draw_info)
> > if (*token == ',')
> > MagickGetToken(q,&q,token,token_max_length);
> > graphic_context[n]->dash_pattern[j]=MagickAtoF(token);
> > + if (graphic_context[n]->dash_pattern[j] < 0.0)
> > + status=MagickFail;
> > }
> ^^^^^^^^^^^^^^^^^^^^
> > + if (status == MagickFail)
> > + {
> > + MagickFreeMemory(graphic_context[n]->dash_pattern);
> > + break;
> > + }
> > if (x & 0x01)
> > for ( ; j < (2*x); j++)
> > graphic_context[n]->dash_pattern[j]=
>
> Attached is the corrected patch.
>
> Philipp "pmhahn@debian.org" Hahn
> --
> Philipp Hahn
> Open Source Software Engineer
>
> Univention GmbH
> be open.
> Mary-Somerville-Str. 1
> D-28359 Bremen
> Tel.: +49 421 22232-0
> Fax : +49 421 22232-99
> hahn@univention.de
>
> http://www.univention.de/
> Geschäftsführer: Peter H. Ganten
> HRB 20755 Amtsgericht Bremen
> Steuer-Nr.: 71-597-02876
> Email had 1 attachment:
> + CVE-2016-5240.patch
> 2k (text/x-diff)
Regards,
--
,''`.
: :' : Chris Lamb
`. `'` lamby@debian.org / chris-lamb.co.uk
`-
Regards,
--
,''`.
: :' : Chris Lamb
`. `'` lamby@debian.org / chris-lamb.co.uk
`-
Reply to: