[Forwarding after getting ACK] ----- Original message ----- From: Chris Lamb <la...@debian.org> To: Philipp Hahn <h...@univention.de>, secur...@debian.org, "Laszlo Boszormenyi (GCS)" <g...@debian.org> Cc: Bob Friesenhahn <bfrie...@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 "pmh...@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 > h...@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 `. `'` la...@debian.org / chris-lamb.co.uk `- Regards, -- ,''`. : :' : Chris Lamb `. `'` la...@debian.org / chris-lamb.co.uk `-