Re: [FFmpeg-devel] [PATCH 6/9] nistspheredec: prevent overflow during block alignment calculation

2017-01-06 Thread Ronald S. Bultje
Hi,

On Fri, Jan 6, 2017 at 5:30 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> On 06.01.2017 22:30, Ronald S. Bultje wrote:
> > On Fri, Jan 6, 2017 at 2:48 PM, Andreas Cadhalpun <
> > andreas.cadhal...@googlemail.com> wrote:
> >
> >> Signed-off-by: Andreas Cadhalpun 
> >> ---
> >>  libavformat/nistspheredec.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
> >> index 782d1dfbfb..9023ed7fc7 100644
> >> --- a/libavformat/nistspheredec.c
> >> +++ b/libavformat/nistspheredec.c
> >> @@ -80,6 +80,7 @@ static int nist_read_header(AVFormatContext *s)
> >>
> >>  avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
> >>
> >> +FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels &&
> >> st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels)
> >
> >
> > Same comment as the other one, the channels == 0 isn't a valid case that
> we
> > want to special case, probably check that it's not zero separately.
>
> Here I disagree: This is only the demuxer, that doesn't need to know
> the number of channels, which can be set later by the decoder.
> (For example the shorten decoder does this.)


Hm ... I don't like how we're adding special-case code for hypothetical
cases that can only come from entirely broken muxers or fuzzers. I'll leave
it to others to break the tie.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 6/9] nistspheredec: prevent overflow during block alignment calculation

2017-01-06 Thread Andreas Cadhalpun
On 06.01.2017 22:30, Ronald S. Bultje wrote:
> On Fri, Jan 6, 2017 at 2:48 PM, Andreas Cadhalpun <
> andreas.cadhal...@googlemail.com> wrote:
> 
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/nistspheredec.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
>> index 782d1dfbfb..9023ed7fc7 100644
>> --- a/libavformat/nistspheredec.c
>> +++ b/libavformat/nistspheredec.c
>> @@ -80,6 +80,7 @@ static int nist_read_header(AVFormatContext *s)
>>
>>  avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
>>
>> +FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels &&
>> st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels)
> 
> 
> Same comment as the other one, the channels == 0 isn't a valid case that we
> want to special case, probably check that it's not zero separately.

Here I disagree: This is only the demuxer, that doesn't need to know
the number of channels, which can be set later by the decoder.
(For example the shorten decoder does this.)

Thus I think this check here is really needed.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 6/9] nistspheredec: prevent overflow during block alignment calculation

2017-01-06 Thread Ronald S. Bultje
Hi,

On Fri, Jan 6, 2017 at 2:48 PM, Andreas Cadhalpun <
andreas.cadhal...@googlemail.com> wrote:

> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/nistspheredec.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
> index 782d1dfbfb..9023ed7fc7 100644
> --- a/libavformat/nistspheredec.c
> +++ b/libavformat/nistspheredec.c
> @@ -80,6 +80,7 @@ static int nist_read_header(AVFormatContext *s)
>
>  avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
>
> +FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels &&
> st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels)


Same comment as the other one, the channels == 0 isn't a valid case that we
want to special case, probably check that it's not zero separately.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH 6/9] nistspheredec: prevent overflow during block alignment calculation

2017-01-06 Thread Andreas Cadhalpun
Signed-off-by: Andreas Cadhalpun 
---
 libavformat/nistspheredec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/nistspheredec.c b/libavformat/nistspheredec.c
index 782d1dfbfb..9023ed7fc7 100644
--- a/libavformat/nistspheredec.c
+++ b/libavformat/nistspheredec.c
@@ -80,6 +80,7 @@ static int nist_read_header(AVFormatContext *s)
 
 avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate);
 
+FF_RETURN_ON_OVERFLOW(s, st->codecpar->channels && 
st->codecpar->bits_per_coded_sample > INT_MAX / st->codecpar->channels)
 st->codecpar->block_align = st->codecpar->bits_per_coded_sample * 
st->codecpar->channels / 8;
 
 if (avio_tell(s->pb) > header_size)
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel