Control: tags -1 + patch Am Mittwoch, den 18.02.2015, 10:53 +0100 schrieb Fabian Greffrath: > But this is still not the cause of the crash, sigh! Patching the sample > to report 1 channel, it still crashes at the same location.
Phew, got it. This time, it was a simple logical error in the lame sources: The "fake" sample rate of the fuzzed input file is 16319999 kHz which lame tries to sample down to 48 kHz in the process of encoding. The ratio between input and output samplerates is thus 16319999/48000=339.999979 which is very close, but only close, to the integer value 340. In libmp3lame/util.c:fill_buffer_resample(), lame checks if the ratio between input and output sample rate is an integer by the following calculation (l. 547): intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) < .0001); Please note that the value of .0001, which the fabs() of the difference is compared against here, is a rather arbitrary value and is *not* sufficient to tell the difference between an integer and a floating point ratio in the case at hand (where it is actually about 0.00002)! The value of "intratio" is added to another variable "filter_l" a few lines later, which in turn is used in the calculation of the value of the "offset" variable, which triggers the assertion (l. 594f): offset = (time0 - esv->itime[ch] - (j + .5 * (filter_l % 2))); assert(fabs(offset) <= .501); In the case at hand, "filter_l" has got an even value by addition of "intratio", which in turn was set to 1 in good faith that the sample rate ratio is integer, whereas in reality it is not. Thus, the latter part of the equation above is not substracted from the "offset" variable, so its value is higher than it should. In the following line, "offset" is used to calculate the value of "joff", which is used to dereference "esv->blackfilt", where it causes an overflow and finally a segfault (l. 608): xvalue += y * esv->blackfilt[joff][i]; The trivial fix for this would be to decrease the arbitrary value of .0001 by another factor 10 and compare against 0.00001, but this would only suffice until the next fuzzed sample with an even higher sample rate is provided. I thus suggest to compare against the smallest number of type double (resample_ratio is of type double) that can still be distinguished from 0: DBL_EPSILON. The attached patch does exact that. Cheers, Fabian -- Dr.-Ing. Fabian Greffrath, Dipl.-Phys. RWTH Aachen University Institute of Mineral Engineering (GHI) Mauerstr. 5, D-52064 Aachen Phone: +49-241-8094979, Fax: +49-241-8092226
Subject: Fix decision if sample rate ratio is an integer value or not If the sample rate of the input file is sufficiently close to an integer multiple of the output sample rate, the value of the intratio variable is calculated incorrectly. This leads to further values being miscalculated up to the joff variable which is used as an index to dereference the esv->blackfilt array. This leads top an overflow and causes a segmentation fault. Author: Fabian Greffrath <fabian+deb...@greffrath.com> Bug-Debian: https://bugs.debian.org/778529 --- a/libmp3lame/util.c +++ b/libmp3lame/util.c @@ -26,6 +26,7 @@ # include <config.h> #endif +#include <float.h> #include "lame.h" #include "machine.h" #include "encoder.h" @@ -544,7 +545,7 @@ fill_buffer_resample(lame_internal_flags if (bpc > BPC) bpc = BPC; - intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) < .0001); + intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) <= DBL_EPSILON); fcn = 1.00 / resample_ratio; if (fcn > 1.00) fcn = 1.00;