Hi Raphael, Thanks for the feedback. On Fri, Oct 28, 2016 at 10:32:06AM +0200, Raphael Hertzog wrote: > Hi, > > On Thu, 27 Oct 2016, Roberto C. Sánchez wrote: > > https://security-tracker.debian.org/tracker/TEMP-0836171-53B142 > > https://bugs.debian.org/836171 > > > > The diff that addresses this issue is here: > > https://github.com/ImageMagick/ImageMagick/commit/10b3823a7619ed22d42764733eb052c4159bc8c1 > > This looks like the wrong diff. And don't forget to add the correct patch > URL to the notes in the tracker. > > https://github.com/ImageMagick/ImageMagick/commit/728dc6a600cf4cbdac846964c85cc04339db8ac1 > You are correct. This is the URL I had in my candidate quilt patch: https://github.com/ImageMagick/ImageMagick/commit/f983dcdf9c178e0cbc49608a78713c5669aa1bb5 That is the cherry-pick of the same commit you referenced. Apparently, when I copy/pasted I had moved on to a different commit on GitHub. > > It is rather short, so I include here as well: > > > > - rows_per_strip=TIFFDefaultStripSize(tiff,0); > > + rows_per_strip=1; > > + if (TIFFScanlineSize(tiff) != 0) > > + rows_per_strip=TIFFDefaultStripSize(tiff,0); > > > > In the wheezy version of ImageMagick, the corresponding section of > > tiff.c looks like this: > > > > rows_per_strip=1; > > if (TIFFScanlineSize(tiff) != 0) > > rows_per_strip=(uint32) MagickMax((size_t) TIFFDefaultStripSize(tiff,0), > > 1); > > > > Naturally, the patch fails to apply. To me it appears that wheezy is > > unaffected by this issue. Perhaps because the code was changed sometime > > after 6.7.7.10 to something less secure and then changed back. My > > instinct is that I do not need to change this section. That being the > > s/instinct/analysis/ hopefully, we are reasoning, not guessing... > > Lacking any file to reproduce the issue, I believe that your analysis > is correct. > Well said. Yes, I did analyse the code and came to my conclusion as a result, though it still felt like there was some amount of instinct involved because I did not have a case which would confirm with certainty that the code in wheezy was unaffected. In any case, it was not a wild guess and I will make sure not create that appearance in the future. > > case, I believe that the correct action would be to add the following in > > data/CVE/list, under "CVE-2016-XXXX [TIFF divide by zero]" near line > > 5702: > > > > [wheezy] - imagemagick <not-affected> (Vulnerable code introduced after 6.7.7.10) > > That's correct. The description is maybe a bit misleading since you have > no certitude. "Vulnerability likely introduced in a later version" > OK, I like your suggestion better so I will use that instead. Regards, -Roberto -- Roberto C. Sánchez http://people.connexer.com/~roberto http://www.connexer.com
Attachment:
signature.asc
Description: Digital signature