Message ID | 20200103155636.7476-7-nicolas.gaullier@cji.paris |
---|---|
State | New |
Headers | show |
Series | avformat: Use s337m subdemux inside wav | expand |
Context | Check | Description |
---|---|---|
andriy/ffmpeg-patchwork | success | Make fate finished |
fre 2020-01-03 klockan 16:56 +0100 skrev Nicolas Gaullier: > --- > libavformat/wavdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c > index 2796905e1f..ccb9576b84 100644 > --- a/libavformat/wavdec.c > +++ b/libavformat/wavdec.c > @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) > ret = AVERROR(ENOMEM); > } else { > int64_t pos = avio_tell(s->pb); > - len = ret = avio_read(s->pb, buf, len); > + len = ret = avio_read(s->pb, buf, FFMIN(len, wav->data_end - pos)); > if (len >= 0) { > ret = ff_spdif_probe(buf, len, &codec); > if (ret > AVPROBE_SCORE_EXTENSION) { Looks OK. I suppose this fixes a potential SIGSEGV too? /Tomas
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index >> 2796905e1f..ccb9576b84 100644 >> --- a/libavformat/wavdec.c >> +++ b/libavformat/wavdec.c >> @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) >> ret = AVERROR(ENOMEM); >> } else { >> int64_t pos = avio_tell(s->pb); >> - len = ret = avio_read(s->pb, buf, len); >> + len = ret = avio_read(s->pb, buf, FFMIN(len, >> + wav->data_end - pos)); >> if (len >= 0) { >> ret = ff_spdif_probe(buf, len, &codec); >> if (ret > AVPROBE_SCORE_EXTENSION) { > >Looks OK. I suppose this fixes a potential SIGSEGV too? AVIO would just stop at EOF, I don't think a SIGSEGV could occur in any scenario. This fix only affects probing (ie. reading start of file) in a surprising scenario where the data chunk would not extend to the end of the file. This is many IF and I find it unlikely, but I think it should be fixed anyway. Nicolas
Am Mo., 13. Jan. 2020 um 15:53 Uhr schrieb Gaullier Nicolas <nicolas.gaullier@cji.paris>: > > >> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index > >> 2796905e1f..ccb9576b84 100644 > >> --- a/libavformat/wavdec.c > >> +++ b/libavformat/wavdec.c > >> @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) > >> ret = AVERROR(ENOMEM); > >> } else { > >> int64_t pos = avio_tell(s->pb); > >> - len = ret = avio_read(s->pb, buf, len); > >> + len = ret = avio_read(s->pb, buf, FFMIN(len, > >> + wav->data_end - pos)); > >> if (len >= 0) { > >> ret = ff_spdif_probe(buf, len, &codec); > >> if (ret > AVPROBE_SCORE_EXTENSION) { > > > >Looks OK. I suppose this fixes a potential SIGSEGV too? > AVIO would just stop at EOF, I don't think a SIGSEGV could occur in any scenario. > This fix only affects probing (ie. reading start of file) in a surprising scenario where the data chunk would not extend to the end of the file. > This is many IF and I find it unlikely, but I think it should be fixed anyway. Could you elaborate? I believe the code is fine as-is but maybe I miss something... Carl Eugen
> > >> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index > > >> 2796905e1f..ccb9576b84 100644 > >> --- a/libavformat/wavdec.c > > >> +++ b/libavformat/wavdec.c > > >> @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) > > >> ret = AVERROR(ENOMEM); > > >> } else { > > >> int64_t pos = avio_tell(s->pb); > > >> - len = ret = avio_read(s->pb, buf, len); > > >> + len = ret = avio_read(s->pb, buf, FFMIN(len, > > >> + wav->data_end - pos)); > > >> if (len >= 0) { > > >> ret = ff_spdif_probe(buf, len, &codec); > > >> if (ret > AVPROBE_SCORE_EXTENSION) { > > > > > >Looks OK. I suppose this fixes a potential SIGSEGV too? > > AVIO would just stop at EOF, I don't think a SIGSEGV could occur in any scenario. > > This fix only affects probing (ie. reading start of file) in a surprising scenario where the data chunk would not extend to the end of the file. > > This is many IF and I find it unlikely, but I think it should be fixed anyway. > > Could you elaborate? Here is a sample a build : http://0x0.st/zhiO.wav This sample is detected as ac3 whereas the sync codes are beyond the data chunk, in a 'junk' (I called it this way) chunk that is not supposed to be read (ex: can be cross-checked with sox or audacity for example). But again, this is obviously very improbable "in the nature" as they are three conditions : - the wav file must be short enough - the data chunk must be followed by an additional header chunk (usually those headers stand before it) - the data chunk must emulate the sync codes Nicolas
diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index 2796905e1f..ccb9576b84 100644 --- a/libavformat/wavdec.c +++ b/libavformat/wavdec.c @@ -78,7 +78,7 @@ static void set_spdif_s337m(AVFormatContext *s, WAVDemuxContext *wav) ret = AVERROR(ENOMEM); } else { int64_t pos = avio_tell(s->pb); - len = ret = avio_read(s->pb, buf, len); + len = ret = avio_read(s->pb, buf, FFMIN(len, wav->data_end - pos)); if (len >= 0) { ret = ff_spdif_probe(buf, len, &codec); if (ret > AVPROBE_SCORE_EXTENSION) {