[vlc-devel] [PATCH] contribs: lame: Add assertion to make sure fftenergy is not read OoB

[Marvin Scholz]

As asserts are disabled in lame, it probably should
be changed to an if () abort(); instead or something
like that?

On 23 Dec 2019, at 15:36, Felix Paul Kühne wrote:

> Hello,
>
> Is there a reason why this wasn’t merged?
>
> Thanks!
>
> Felix
>
>> On 2. Dec 2019, at 10:37, david.fuhrmann at gmail.com wrote:
>>
>> From: David Fuhrmann <dfuhrmann at videolan.org>
>>
>> On macOS (only), we see crashes on the assertion a couple of code
>> lines below:
>>
>> FLOAT const el = fftenergy[j];
>> assert(el >= 0);
>>
>> To narrow down the problem a bit, add a new assertion to make sure
>> fftenergy array is never read past its bound, which would lead to
>> undefined and potentially negative values.
>> ---
>> contrib/src/lame/invariant-for-energy-array.patch | 11 +++++++++++
>> contrib/src/lame/rules.mak                        |  4 ++++
>> 2 files changed, 15 insertions(+)
>> create mode 100644 contrib/src/lame/invariant-for-energy-array.patch
>>
>> diff --git a/contrib/src/lame/invariant-for-energy-array.patch 
>> b/contrib/src/lame/invariant-for-energy-array.patch
>> new file mode 100644
>> index 000000000000..392a40380a5e
>> --- /dev/null
>> +++ b/contrib/src/lame/invariant-for-energy-array.patch
>> @@ -0,0 +1,11 @@
>> +--- lame/libmp3lame/psymodel.c.orig	2019-12-02 10:13:52.000000000 
>> +0100
>> ++++ lame/libmp3lame/psymodel.c	2019-12-02 10:14:53.000000000 +0100
>> +@@ -571,6 +571,8 @@
>> +     for (b = j = 0; b < l->npart; ++b) {
>> +         FLOAT   ebb = 0, m = 0;
>> +         int     i;
>> ++
>> ++        assert(j + l->numlines[b] <= HBLKSIZE);
>> +         for (i = 0; i < l->numlines[b]; ++i, ++j) {
>> +             FLOAT const el = fftenergy[j];
>> +             assert(el >= 0);
>> diff --git a/contrib/src/lame/rules.mak b/contrib/src/lame/rules.mak
>> index 32827a8e69ac..50ce55625f89 100644
>> --- a/contrib/src/lame/rules.mak
>> +++ b/contrib/src/lame/rules.mak
>> @@ -15,6 +15,10 @@ lame: lame-$(LAME_VERSION).tar.gz .sum-lame
>> ifdef HAVE_VISUALSTUDIO
>> 	$(APPLY) $(SRC)/lame/struct-float-copy.patch
>> endif
>> +ifdef HAVE_DARWIN_OS
>> +	$(APPLY) $(SRC)/lame/invariant-for-energy-array.patch
>> +endif
>> +
>> 	# Avoid relying on iconv.m4 from gettext, when reconfiguring.
>> 	# This is only used by the frontend which we disable.
>> 	cd $(UNPACK_DIR) && sed -i.orig 's/^AM_ICONV/#&/' configure.in
>> -- 
>> 2.21.0 (Apple Git-122.2)
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel