Division by zero in GenerateDifferentialNoise() in MagickCore/gem.c

[hifoolno]

When attenuate was set to zero, SigmaPoisson would be zero too. So a crafted file may trigger undefined behavior in the form of division by zero. Maybe there need a check on SigmaPoisson before line 1590?

ImageMagick/MagickCore/gem.c

Lines 1576 to 1592 in 0d20727

	case PoissonNoise:
	{
	double
	poisson;
	ssize_t
	i;
	poisson=exp(-SigmaPoisson*QuantumScale*pixel);
	for (i=0; alpha > poisson; i++)
	{
	beta=GetPseudoRandomValue(random_info);
	alpha*=beta;
	}
	noise=(double) (QuantumRange*i/SigmaPoisson);
	break;
	}

[urban-warrior]

Thanks for the problem report. We can reproduce it and will have a patch to fix it in the GIT master branch @ https://github.com/ImageMagick/ImageMagick later today. The patch will be available in the beta releases of ImageMagick @ https://www.imagemagick.org/download/beta/ by sometime tomorrow.

[carnil]

This issue was assigned CVE-2021-20176

[StayPirate]

Hi @urban-warrior, do you think the following lines are relevant for this vulnerability?

ImageMagick:

ImageMagick/MagickCore/accelerate-kernels-private.h

Line 812 in d130955

noise=(QuantumRange*i/SigmaPoisson);

ImageMagick6:
https://github.com/ImageMagick/ImageMagick6/blob/210e4177f70a2ab4b5138c1453e50c9384d1626e/magick/accelerate-kernels-private.h#L706

[urban-warrior]

Yes. We'll add a patch.

[StayPirate]

can you please explain me why there are two different commits addressing the same line of code?? 4103225 and fbd9a96

[urban-warrior]

We maintain two different releases of ImageMagick. IMv6 & IMv7.

[StayPirate]

Yes, but these are both for IMv7. IMv6 is in a different repo, am I wrong?

[StayPirate]

I got it, you merged 4103225 in the a0d7cbc, resulting in an empty merge. That's because the same changes were applied by fbd9a96 the 7th of Jan.