Re: [FFmpeg-devel] [PATCH v2 7/8] avformat/wavdec: fix s337m/spdif probing beyond data_end
Am Mi., 15. Jan. 2020 um 16:07 Uhr schrieb Gaullier Nicolas
:
>
> >>
> >> ---
> >> libavformat/wavdec.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
> >> 3571733817..d8a27c79cf 100644
> >> --- a/libavformat/wavdec.c
> >> +++ b/libavformat/wavdec.c
> >> @@ -77,7 +77,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));
> >
> >Sorry if this was already answered:
> >What exactly does this fix? Is it possible that avio_read() overreads
> >without this check?
> >
> >Carl Eugen
>
> There is no severe low level avio overread risk, but there is a risk to read
> bytes that are not audio data in case audio data is followed by other data (a
> RIFF chunk that would come just after the audio data RIFF chunk).
>
> 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: the duration 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 (for the probe to seek in the end of the
> file)
> - the audio data chunk must be followed by another chunk with misc data
> (usually those are considered "header metadata" and stand before it)
> - the misc data chunk must emulate the sync codes
Thank you for the explanation.
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 7/8] avformat/wavdec: fix s337m/spdif probing beyond data_end
>>
>> ---
>> libavformat/wavdec.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c index
>> 3571733817..d8a27c79cf 100644
>> --- a/libavformat/wavdec.c
>> +++ b/libavformat/wavdec.c
>> @@ -77,7 +77,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));
>
>Sorry if this was already answered:
>What exactly does this fix? Is it possible that avio_read() overreads without
>this check?
>
>Carl Eugen
There is no severe low level avio overread risk, but there is a risk to read
bytes that are not audio data in case audio data is followed by other data (a
RIFF chunk that would come just after the audio data RIFF chunk).
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: the duration 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 (for the probe to seek in the end of the
file)
- the audio data chunk must be followed by another chunk with misc data
(usually those are considered "header metadata" and stand before it)
- the misc data chunk must emulate the sync codes
Nicolas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 7/8] avformat/wavdec: fix s337m/spdif probing beyond data_end
Am Mi., 15. Jan. 2020 um 11:56 Uhr schrieb Nicolas Gaullier
:
>
> ---
> libavformat/wavdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> index 3571733817..d8a27c79cf 100644
> --- a/libavformat/wavdec.c
> +++ b/libavformat/wavdec.c
> @@ -77,7 +77,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));
Sorry if this was already answered:
What exactly does this fix? Is it possible that avio_read() overreads without
this check?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 7/8] avformat/wavdec: fix s337m/spdif probing beyond data_end
---
libavformat/wavdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index 3571733817..d8a27c79cf 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -77,7 +77,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) {
--
2.14.1.windows.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
