Bug#778529: lame: fill_buffer_resample segmentation fault

[Fabian Greffrath]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: int_resample_ratio.patch
Type: text/x-patch
Size: 1067 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20150218/db9d1e3c/attachment.bin>