diff mbox

[FFmpeg-devel,3/9] electronicarts: prevent overflow during block alignment calculation

Message ID 29837586-a59c-56e7-5b0b-61c1edadedb0@googlemail.com
State Accepted
Headers show

Commit Message

Andreas Cadhalpun Jan. 6, 2017, 7:47 p.m. UTC
Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---
 libavformat/electronicarts.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Michael Niedermayer Jan. 7, 2017, 1:37 a.m. UTC | #1
On Fri, Jan 06, 2017 at 08:47:39PM +0100, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>  libavformat/electronicarts.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
> index 30eb723bd5..03422e5b2c 100644
> --- a/libavformat/electronicarts.c
> +++ b/libavformat/electronicarts.c
> @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s)
>          st->codecpar->codec_tag             = 0;   /* no tag */
>          st->codecpar->channels              = ea->num_channels;
>          st->codecpar->sample_rate           = ea->sample_rate;
> +        FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)

I think we should ask for a sample when the number of bytes per
sample is larger than 2 or 4 or whatever max we think occurs.

the patch is probably fine

[...]
Paul B Mahol Jan. 7, 2017, 8:32 a.m. UTC | #2
On 1/7/17, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Jan 06, 2017 at 08:47:39PM +0100, Andreas Cadhalpun wrote:
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
>> ---
>>  libavformat/electronicarts.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
>> index 30eb723bd5..03422e5b2c 100644
>> --- a/libavformat/electronicarts.c
>> +++ b/libavformat/electronicarts.c
>> @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s)
>>          st->codecpar->codec_tag             = 0;   /* no tag */
>>          st->codecpar->channels              = ea->num_channels;
>>          st->codecpar->sample_rate           = ea->sample_rate;
>> +        FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)
>
> I think we should ask for a sample when the number of bytes per
> sample is larger than 2 or 4 or whatever max we think occurs.

No we should not as such samples are invalid.

>
> the patch is probably fine
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I am the wisest man alive, for I know one thing, and that is that I know
> nothing. -- Socrates
>
Andreas Cadhalpun Jan. 26, 2017, 1:09 a.m. UTC | #3
On 07.01.2017 09:32, Paul B Mahol wrote:
> On 1/7/17, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> On Fri, Jan 06, 2017 at 08:47:39PM +0100, Andreas Cadhalpun wrote:
>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
>>> ---
>>>  libavformat/electronicarts.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
>>> index 30eb723bd5..03422e5b2c 100644
>>> --- a/libavformat/electronicarts.c
>>> +++ b/libavformat/electronicarts.c
>>> @@ -556,6 +556,7 @@ static int ea_read_header(AVFormatContext *s)
>>>          st->codecpar->codec_tag             = 0;   /* no tag */
>>>          st->codecpar->channels              = ea->num_channels;
>>>          st->codecpar->sample_rate           = ea->sample_rate;
>>> +        FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)
>>
>> I think we should ask for a sample when the number of bytes per
>> sample is larger than 2 or 4 or whatever max we think occurs.
> 
> No we should not as such samples are invalid.

The code seems to only know about 1 (8-bit) and 2 (16-bit), so
returning an error for larger values makes sense.

Best regards,
Andreas
diff mbox

Patch

diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index 30eb723bd5..03422e5b2c 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -556,6 +556,7 @@  static int ea_read_header(AVFormatContext *s)
         st->codecpar->codec_tag             = 0;   /* no tag */
         st->codecpar->channels              = ea->num_channels;
         st->codecpar->sample_rate           = ea->sample_rate;
+        FF_RETURN_ON_OVERFLOW(s, ea->bytes > INT_MAX / 8 / 2)
         st->codecpar->bits_per_coded_sample = ea->bytes * 8;
         st->codecpar->bit_rate              = (int64_t)st->codecpar->channels *
                                               st->codecpar->sample_rate *