Buffer overrun in III_dequantize_sample

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Buffer overrun in III_dequantize_sample

electricworry
Hello,

I've been doing some testing of the latest LAME in CVS and there's the
possibility to cause a buffer overrun in III_dequantize_sample by inputting
a malformed file. It would appear to not be exploitable, but it should
probably be fixed.

Unfortunately I'm not an audio developer and a lot of the code is
mysterious to me. I'm therefore having trouble identifying the root cause.
(e.g. I don't know whether there simply needs to be a bounds check in the
affected function, or whether there is an initialisation issue further down
the call stack that needs to be addressed.)

It would appear that the issue occurs when:

sfreq == 3;
gr_infos->block_type == 2
gr_infos->mixed_block_flag is true

However, there might be additional factors. I have an input file that
reproduces the issue. Would anyone be able to help me identify the root
cause?
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lame-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/lame-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overrun in III_dequantize_sample

Thomas Orgis
Am Fri, 21 Jul 2017 10:23:45 +0100
schrieb electricworry <[hidden email]>:

> I've been doing some testing of the latest LAME in CVS and there's the
> possibility to cause a buffer overrun in III_dequantize_sample by inputting
> a malformed file. It would appear to not be exploitable, but it should
> probably be fixed.

Is this different from

        https://blogs.gentoo.org/ago/2017/06/17/lame-stack-based-buffer-overflow-in-iii_dequantize_sample-layer3-c

? Do you produce a crash with/without ASan?

> sfreq == 3;
> gr_infos->block_type == 2
> gr_infos->mixed_block_flag is true

I guess that is not the full set of conditions …

> However, there might be additional factors. I have an input file that
> reproduces the issue. Would anyone be able to help me identify the root
> cause?

As mpg123 developer, I am muchly interested in your input file to see
if it also triggers that. The respective code in LAME is a fork of
mpg123 and the fix is likely identical. Can you send the example to
[hidden email]? I guess the first n KiB of the file can be enough.

In any case … we should go forward with the plan to remove the decoding
code from lame and install additional hooks in libmpg123 to be able to
build the analyser. The trouble is to define the “we” here.


Alrighty then,

Thomas
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lame-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/lame-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overrun in III_dequantize_sample

electricworry
On 21 July 2017 at 16:07, Thomas Orgis <[hidden email]> wrote:
> ? Do you produce a crash with/without ASan?

No, this test case (and AFL/ASan has found a bunch of others) only
crashes with ASan. However, it I do a normal build with
-gno-stack-protector, some of the other test cases do produce a seg
fault.

But to stick to this one case, no; no crash is produced without ASan.

> Is this different from
>
>         https://blogs.gentoo.org/ago/2017/06/17/lame-stack-based-buffer-overflow-in-iii_dequantize_sample-layer3-c

It might be; it's certainly got a near identical stack trace. However,
when I run the test case linked to, I don't get a crash.

> I guess that is not the full set of conditions …

Yes, I feel my lack of knowledge about implementation is unhelpful.

> As mpg123 developer, I am muchly interested in your input file to see
> if it also triggers that. The respective code in LAME is a fork of
> mpg123 and the fix is likely identical. Can you send the example to
> [hidden email]? I guess the first n KiB of the file can be enough.
>
> In any case … we should go forward with the plan to remove the decoding
> code from lame and install additional hooks in libmpg123 to be able to
> build the analyser. The trouble is to define the “we” here.

I'm happy to send the example over and will do so now.

As I mentioned earlier, testing has found a bunch of crash cases in
various other functions. Some of them are further stack buffer
overflows, some are global buffer overflows. However I didn't want to
mention more than one to start with, as they might have the same root
cause. If you'd like them, just shout.

I'm sorry that I'm not able to offer better assistance, not being an
audio developer. I'm assuming that these are not exploitable and very
low urgency.

Thanks.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lame-dev mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/lame-dev