[06/17] mvi: Avoid a division by zero

Message ID 1380406879-6174-6-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Sept. 28, 2013, 10:21 p.m.
Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
CC: libav-stable@libav.org
---
 libavformat/mvi.c |    6 ++++++
 1 file changed, 6 insertions(+)

Comments

Kostya Shishkov Sept. 29, 2013, 6 a.m. | #1
On Sun, Sep 29, 2013 at 01:21:08AM +0300, Martin Storsjö wrote:
> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> CC: libav-stable@libav.org
> ---
>  libavformat/mvi.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavformat/mvi.c b/libavformat/mvi.c
> index 7fb163b..af7b5c5 100644
> --- a/libavformat/mvi.c
> +++ b/libavformat/mvi.c
> @@ -94,6 +94,12 @@ static int read_header(AVFormatContext *s)
>      mvi->get_int = (vst->codec->width * vst->codec->height < (1 << 16)) ? avio_rl16 : avio_rl24;
>  
>      mvi->audio_frame_size   = ((uint64_t)mvi->audio_data_size << MVI_FRAC_BITS) / frames_count;
> +    if (mvi->audio_frame_size <= 1) {
> +        av_log(s, AV_LOG_ERROR, "Invalid audio_data_size (%d) or frames_count (%d)\n",
> +               mvi->audio_data_size, frames_count);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
>      mvi->audio_size_counter = (ast->codec->sample_rate * 830 / mvi->audio_frame_size - 1) * mvi->audio_frame_size;
>      mvi->audio_size_left    = mvi->audio_data_size;
>  
> -- 

hmm, a quick look at demuxer suggests that minimal frame size should be more,
e.g. 512 (1 << MVI_FRAC_BITS - 1)
Martin Storsjö Sept. 29, 2013, 9:40 a.m. | #2
On Sun, 29 Sep 2013, Kostya Shishkov wrote:

> On Sun, Sep 29, 2013 at 01:21:08AM +0300, Martin Storsjö wrote:
>> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
>> CC: libav-stable@libav.org
>> ---
>>  libavformat/mvi.c |    6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/libavformat/mvi.c b/libavformat/mvi.c
>> index 7fb163b..af7b5c5 100644
>> --- a/libavformat/mvi.c
>> +++ b/libavformat/mvi.c
>> @@ -94,6 +94,12 @@ static int read_header(AVFormatContext *s)
>>      mvi->get_int = (vst->codec->width * vst->codec->height < (1 << 16)) ? avio_rl16 : avio_rl24;
>>
>>      mvi->audio_frame_size   = ((uint64_t)mvi->audio_data_size << MVI_FRAC_BITS) / frames_count;
>> +    if (mvi->audio_frame_size <= 1) {
>> +        av_log(s, AV_LOG_ERROR, "Invalid audio_data_size (%d) or frames_count (%d)\n",
>> +               mvi->audio_data_size, frames_count);
>> +        return AVERROR_INVALIDDATA;
>> +    }
>> +
>>      mvi->audio_size_counter = (ast->codec->sample_rate * 830 / mvi->audio_frame_size - 1) * mvi->audio_frame_size;
>>      mvi->audio_size_left    = mvi->audio_data_size;
>>
>> --
>
> hmm, a quick look at demuxer suggests that minimal frame size should be more,
> e.g. 512 (1 << MVI_FRAC_BITS - 1)

That does indeed seem to be correct, I'll raise the limit to that.

// Martin
Kostya Shishkov Sept. 29, 2013, 11:05 a.m. | #3
On Sun, Sep 29, 2013 at 12:40:28PM +0300, Martin Storsjö wrote:
> On Sun, 29 Sep 2013, Kostya Shishkov wrote:
> 
> >On Sun, Sep 29, 2013 at 01:21:08AM +0300, Martin Storsjö wrote:
> >>Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> >>CC: libav-stable@libav.org
> >>---
> >> libavformat/mvi.c |    6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >>diff --git a/libavformat/mvi.c b/libavformat/mvi.c
> >>index 7fb163b..af7b5c5 100644
> >>--- a/libavformat/mvi.c
> >>+++ b/libavformat/mvi.c
> >>@@ -94,6 +94,12 @@ static int read_header(AVFormatContext *s)
> >>     mvi->get_int = (vst->codec->width * vst->codec->height < (1 << 16)) ? avio_rl16 : avio_rl24;
> >>
> >>     mvi->audio_frame_size   = ((uint64_t)mvi->audio_data_size << MVI_FRAC_BITS) / frames_count;
> >>+    if (mvi->audio_frame_size <= 1) {
> >>+        av_log(s, AV_LOG_ERROR, "Invalid audio_data_size (%d) or frames_count (%d)\n",
> >>+               mvi->audio_data_size, frames_count);
> >>+        return AVERROR_INVALIDDATA;
> >>+    }
> >>+
> >>     mvi->audio_size_counter = (ast->codec->sample_rate * 830 / mvi->audio_frame_size - 1) * mvi->audio_frame_size;
> >>     mvi->audio_size_left    = mvi->audio_data_size;
> >>
> >>--
> >
> >hmm, a quick look at demuxer suggests that minimal frame size should be more,
> >e.g. 512 (1 << MVI_FRAC_BITS - 1)
> 
> That does indeed seem to be correct, I'll raise the limit to that.

and it should be good enough then

Patch

diff --git a/libavformat/mvi.c b/libavformat/mvi.c
index 7fb163b..af7b5c5 100644
--- a/libavformat/mvi.c
+++ b/libavformat/mvi.c
@@ -94,6 +94,12 @@  static int read_header(AVFormatContext *s)
     mvi->get_int = (vst->codec->width * vst->codec->height < (1 << 16)) ? avio_rl16 : avio_rl24;
 
     mvi->audio_frame_size   = ((uint64_t)mvi->audio_data_size << MVI_FRAC_BITS) / frames_count;
+    if (mvi->audio_frame_size <= 1) {
+        av_log(s, AV_LOG_ERROR, "Invalid audio_data_size (%d) or frames_count (%d)\n",
+               mvi->audio_data_size, frames_count);
+        return AVERROR_INVALIDDATA;
+    }
+
     mvi->audio_size_counter = (ast->codec->sample_rate * 830 / mvi->audio_frame_size - 1) * mvi->audio_frame_size;
     mvi->audio_size_left    = mvi->audio_data_size;