diff mbox

[FFmpeg-devel] lavc/alac: Avoid allocating huge memory blocks for malicious alac input.

Message ID CAB0OVGpVzWpKxYB4gfM4A8L78HAvxcHiAn0Yb_w3qR4ZmSnr6g@mail.gmail.com
State Accepted
Headers show

Commit Message

Carl Eugen Hoyos Nov. 1, 2017, 2:25 p.m. UTC
Hi!

It appears to me that the alac decoder can be used for DoS, the attached patch
limits the maximum frame size to eight times the default value.
(Higher values brake our encoder here.)

Please comment and / or suggest another value, Carl Eugen

Comments

Paul B Mahol Nov. 1, 2017, 2:40 p.m. UTC | #1
On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> Hi!
>
> It appears to me that the alac decoder can be used for DoS, the attached
> patch
> limits the maximum frame size to eight times the default value.
> (Higher values brake our encoder here.)
>
> Please comment and / or suggest another value, Carl Eugen
>

So alac encoder can not handle bigger frames or what?

Look at other alac encoders, what are their limit on frame size?

The limit you set is too low IMHO.
Derek Buitenhuis Nov. 1, 2017, 2:46 p.m. UTC | #2
On 11/1/2017 2:25 PM, Carl Eugen Hoyos wrote:
> It appears to me that the alac decoder can be used for DoS, the attached patch
> limits the maximum frame size to eight times the default value.
> (Higher values brake our encoder here.)

Since the official ALAC encoder/decoder are open ource nowadays, I took a look
a its source, and it doesn't seem to set any such limit in the encoder or decoder.

So, isn't it possible this arbitrary hardcoded limit breaks valid files?

- Derek
Carl Eugen Hoyos Nov. 1, 2017, 2:50 p.m. UTC | #3
2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> Hi!
>>
>> It appears to me that the alac decoder can be used for DoS,
>> the attached patch limits the maximum frame size to eight
>> times the default value.
>> (Higher values brake our encoder here.)
>>
>> Please comment and / or suggest another value, Carl Eugen
>>
>
> So alac encoder can not handle bigger frames or what?
>
> Look at other alac encoders, what are their limit on frame size?

I am not sure if it is enough to look on Apple's encoder, after
all, their decoder looks exploitable (or maybe I miss something).

> The limit you set is too low IMHO.

Could you suggest a limit that's below the several-GB area?

Thank you, Carl Eugen
Paul B Mahol Nov. 1, 2017, 4:01 p.m. UTC | #4
On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> Hi!
>>>
>>> It appears to me that the alac decoder can be used for DoS,
>>> the attached patch limits the maximum frame size to eight
>>> times the default value.
>>> (Higher values brake our encoder here.)
>>>
>>> Please comment and / or suggest another value, Carl Eugen
>>>
>>
>> So alac encoder can not handle bigger frames or what?
>>
>> Look at other alac encoders, what are their limit on frame size?
>
> I am not sure if it is enough to look on Apple's encoder, after
> all, their decoder looks exploitable (or maybe I miss something).
>
>> The limit you set is too low IMHO.
>
> Could you suggest a limit that's below the several-GB area?

I remmeber some lossless audio codecs can have very big frames, several MB.
Carl Eugen Hoyos Nov. 1, 2017, 4:03 p.m. UTC | #5
2017-11-01 17:01 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>> Hi!
>>>>
>>>> It appears to me that the alac decoder can be used for DoS,
>>>> the attached patch limits the maximum frame size to eight
>>>> times the default value.
>>>> (Higher values brake our encoder here.)
>>>>
>>>> Please comment and / or suggest another value, Carl Eugen
>>>>
>>>
>>> So alac encoder can not handle bigger frames or what?
>>>
>>> Look at other alac encoders, what are their limit on frame size?
>>
>> I am not sure if it is enough to look on Apple's encoder, after
>> all, their decoder looks exploitable (or maybe I miss something).
>>
>>> The limit you set is too low IMHO.
>>
>> Could you suggest a limit that's below the several-GB area?
>
> I remmeber some lossless audio codecs can have very big
> frames, several MB.

So what about 4096 * 4096 as an arbitrary limit?

Carl Eugen
Carl Eugen Hoyos Nov. 4, 2017, 12:05 a.m. UTC | #6
2017-11-01 17:03 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2017-11-01 17:01 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> 2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>> Hi!
>>>>>
>>>>> It appears to me that the alac decoder can be used for DoS,
>>>>> the attached patch limits the maximum frame size to eight
>>>>> times the default value.
>>>>> (Higher values brake our encoder here.)
>>>>>
>>>>> Please comment and / or suggest another value, Carl Eugen
>>>>>
>>>>
>>>> So alac encoder can not handle bigger frames or what?
>>>>
>>>> Look at other alac encoders, what are their limit on frame size?
>>>
>>> I am not sure if it is enough to look on Apple's encoder, after
>>> all, their decoder looks exploitable (or maybe I miss something).
>>>
>>>> The limit you set is too low IMHO.
>>>
>>> Could you suggest a limit that's below the several-GB area?
>>
>> I remmeber some lossless audio codecs can have very big
>> frames, several MB.
>
> So what about 4096 * 4096 as an arbitrary limit?

Any opinion?

Carl Eugen
Paul B Mahol Nov. 4, 2017, 9:23 a.m. UTC | #7
On 11/4/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2017-11-01 17:03 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>> 2017-11-01 17:01 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>> 2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>>> Hi!
>>>>>>
>>>>>> It appears to me that the alac decoder can be used for DoS,
>>>>>> the attached patch limits the maximum frame size to eight
>>>>>> times the default value.
>>>>>> (Higher values brake our encoder here.)
>>>>>>
>>>>>> Please comment and / or suggest another value, Carl Eugen
>>>>>>
>>>>>
>>>>> So alac encoder can not handle bigger frames or what?
>>>>>
>>>>> Look at other alac encoders, what are their limit on frame size?
>>>>
>>>> I am not sure if it is enough to look on Apple's encoder, after
>>>> all, their decoder looks exploitable (or maybe I miss something).
>>>>
>>>>> The limit you set is too low IMHO.
>>>>
>>>> Could you suggest a limit that's below the several-GB area?
>>>
>>> I remmeber some lossless audio codecs can have very big
>>> frames, several MB.
>>
>> So what about 4096 * 4096 as an arbitrary limit?
>
> Any opinion?

ok
Carl Eugen Hoyos Nov. 4, 2017, 4:09 p.m. UTC | #8
2017-11-04 10:23 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> On 11/4/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2017-11-01 17:03 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
>>> 2017-11-01 17:01 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>> 2017-11-01 15:40 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
>>>>>> On 11/1/17, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> It appears to me that the alac decoder can be used for DoS,
>>>>>>> the attached patch limits the maximum frame size to eight
>>>>>>> times the default value.
>>>>>>> (Higher values brake our encoder here.)
>>>>>>>
>>>>>>> Please comment and / or suggest another value, Carl Eugen
>>>>>>>
>>>>>>
>>>>>> So alac encoder can not handle bigger frames or what?
>>>>>>
>>>>>> Look at other alac encoders, what are their limit on frame size?
>>>>>
>>>>> I am not sure if it is enough to look on Apple's encoder, after
>>>>> all, their decoder looks exploitable (or maybe I miss something).
>>>>>
>>>>>> The limit you set is too low IMHO.
>>>>>
>>>>> Could you suggest a limit that's below the several-GB area?
>>>>
>>>> I remmeber some lossless audio codecs can have very big
>>>> frames, several MB.
>>>
>>> So what about 4096 * 4096 as an arbitrary limit?
>>
>> Any opinion?
>
> ok

Patch applied.

Thank you, Carl Eugen
diff mbox

Patch

From c2181c7ee83fcf93ba817cf6f9c3c9e1043a233c Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Wed, 1 Nov 2017 15:14:22 +0100
Subject: [PATCH] lavc/alac: Avoid allocating huge memory blocks for malicious
 alac input.

---
 libavcodec/alac.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/alac.c b/libavcodec/alac.c
index d6bd21b..66bee7f 100644
--- a/libavcodec/alac.c
+++ b/libavcodec/alac.c
@@ -524,7 +524,7 @@  static int alac_set_info(ALACContext *alac)
 
     alac->max_samples_per_frame = bytestream2_get_be32u(&gb);
     if (!alac->max_samples_per_frame ||
-        alac->max_samples_per_frame > INT_MAX / sizeof(int32_t)) {
+        alac->max_samples_per_frame > 4096 * 16) {
         av_log(alac->avctx, AV_LOG_ERROR,
                "max samples per frame invalid: %"PRIu32"\n",
                alac->max_samples_per_frame);
-- 
1.7.10.4