| Message ID | 1380406879-6174-7-git-send-email-martin@martin.st |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Sep 29, 2013 at 01:21:09AM +0300, Martin Storsjö wrote: > Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind > CC: libav-stable@libav.org > --- > libavformat/riffdec.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c > index 447a686..1927b82 100644 > --- a/libavformat/riffdec.c > +++ b/libavformat/riffdec.c > @@ -127,8 +127,14 @@ int ff_get_wav_header(AVIOContext *pb, AVCodecContext *codec, int size) > codec->sample_rate = 0; > } > /* override bits_per_coded_sample for G.726 */ > - if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) > + if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) { > + if (codec->sample_rate <= 0) { > + av_log(NULL, AV_LOG_ERROR, > + "Invalid sample rate for G726: %d\n", codec->sample_rate); > + return AVERROR_INVALIDDATA; > + } > codec->bits_per_coded_sample = codec->bit_rate / codec->sample_rate; > + } > > return 0; > } > -- this makes me wonder why we should allow such sample rates for other codecs
On Sun, 29 Sep 2013, Kostya Shishkov wrote: > On Sun, Sep 29, 2013 at 01:21:09AM +0300, Martin Storsjö wrote: >> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind >> CC: libav-stable@libav.org >> --- >> libavformat/riffdec.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c >> index 447a686..1927b82 100644 >> --- a/libavformat/riffdec.c >> +++ b/libavformat/riffdec.c >> @@ -127,8 +127,14 @@ int ff_get_wav_header(AVIOContext *pb, AVCodecContext *codec, int size) >> codec->sample_rate = 0; >> } >> /* override bits_per_coded_sample for G.726 */ >> - if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) >> + if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) { >> + if (codec->sample_rate <= 0) { >> + av_log(NULL, AV_LOG_ERROR, >> + "Invalid sample rate for G726: %d\n", codec->sample_rate); >> + return AVERROR_INVALIDDATA; >> + } >> codec->bits_per_coded_sample = codec->bit_rate / codec->sample_rate; >> + } >> >> return 0; >> } >> -- > > this makes me wonder why we should allow such sample rates for other codecs I thought about this as well, and I guess we could try to check it for all cases. There's a special case for AAC_LATM where we intentionally set it to zero, but I guess we could place the check before that (and only allow it to be left at zero in that case). Or would this break any other known case where it's ok to have it set to zero in the wav header? (It passes all fate tests but that doesn't tell half the story of how it's used in practice...) // Martin
diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c index 447a686..1927b82 100644 --- a/libavformat/riffdec.c +++ b/libavformat/riffdec.c @@ -127,8 +127,14 @@ int ff_get_wav_header(AVIOContext *pb, AVCodecContext *codec, int size) codec->sample_rate = 0; } /* override bits_per_coded_sample for G.726 */ - if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) + if (codec->codec_id == AV_CODEC_ID_ADPCM_G726) { + if (codec->sample_rate <= 0) { + av_log(NULL, AV_LOG_ERROR, + "Invalid sample rate for G726: %d\n", codec->sample_rate); + return AVERROR_INVALIDDATA; + } codec->bits_per_coded_sample = codec->bit_rate / codec->sample_rate; + } return 0; }