| Message ID | 20200606180815.12736-2-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/oggdec: Initialize return value from ogg_read_page() and check it everywhere | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Jun 6, 2020, 19:08 by michael@niedermayer.cc: > Fixes: regression since e983197cbc93420b67aa7e811be47d7278c2c8a2 > Fixes: out of array read > Fixes: 22185/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5662069073641472 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/oggdec.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > index f65013f55e..bd4311bb41 100644 > --- a/libavformat/oggdec.c > +++ b/libavformat/oggdec.c > @@ -206,7 +206,7 @@ static const struct ogg_codec *ogg_find_codec(uint8_t *buf, int size) > * situation where a new audio stream spawn (identified with a new serial) and > * must replace the previous one (track switch). > */ > -static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > +static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, int magic_size, > int probing) > { > struct ogg *ogg = s->priv_data; > @@ -220,7 +220,7 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > } > > /* Check for codecs */ > - codec = ogg_find_codec(magic, 8); > + codec = ogg_find_codec(magic, magic_size); > if (!codec && !probing) { > av_log(s, AV_LOG_ERROR, "Cannot identify new stream\n"); > return AVERROR_INVALIDDATA; > @@ -430,7 +430,7 @@ static int ogg_read_page(AVFormatContext *s, int *sid, int probing) > /* CRC is correct so we can be 99% sure there's an actual change here */ > if (idx < 0) { > if (data_packets_seen(ogg)) > - idx = ogg_replace_stream(s, serial, readout_buf, probing); > + idx = ogg_replace_stream(s, serial, readout_buf, size, probing); > else > idx = ogg_new_stream(s, serial); > Makes sense. Just rename it from magic_size to page_size. Apart from that LGTM.
On Sat, Jun 06, 2020 at 08:39:15PM +0200, Lynne wrote: > Jun 6, 2020, 19:08 by michael@niedermayer.cc: > > > Fixes: regression since e983197cbc93420b67aa7e811be47d7278c2c8a2 > > Fixes: out of array read > > Fixes: 22185/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5662069073641472 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/oggdec.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c > > index f65013f55e..bd4311bb41 100644 > > --- a/libavformat/oggdec.c > > +++ b/libavformat/oggdec.c > > @@ -206,7 +206,7 @@ static const struct ogg_codec *ogg_find_codec(uint8_t *buf, int size) > > * situation where a new audio stream spawn (identified with a new serial) and > > * must replace the previous one (track switch). > > */ > > -static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > > +static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, int magic_size, > > int probing) > > { > > struct ogg *ogg = s->priv_data; > > @@ -220,7 +220,7 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, > > } > > > > /* Check for codecs */ > > - codec = ogg_find_codec(magic, 8); > > + codec = ogg_find_codec(magic, magic_size); > > if (!codec && !probing) { > > av_log(s, AV_LOG_ERROR, "Cannot identify new stream\n"); > > return AVERROR_INVALIDDATA; > > @@ -430,7 +430,7 @@ static int ogg_read_page(AVFormatContext *s, int *sid, int probing) > > /* CRC is correct so we can be 99% sure there's an actual change here */ > > if (idx < 0) { > > if (data_packets_seen(ogg)) > > - idx = ogg_replace_stream(s, serial, readout_buf, probing); > > + idx = ogg_replace_stream(s, serial, readout_buf, size, probing); > > else > > idx = ogg_new_stream(s, serial); > > > > Makes sense. > Just rename it from magic_size to page_size. Apart from that LGTM. variable renamed, will apply thanks [...]
diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index f65013f55e..bd4311bb41 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -206,7 +206,7 @@ static const struct ogg_codec *ogg_find_codec(uint8_t *buf, int size) * situation where a new audio stream spawn (identified with a new serial) and * must replace the previous one (track switch). */ -static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, +static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, int magic_size, int probing) { struct ogg *ogg = s->priv_data; @@ -220,7 +220,7 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, } /* Check for codecs */ - codec = ogg_find_codec(magic, 8); + codec = ogg_find_codec(magic, magic_size); if (!codec && !probing) { av_log(s, AV_LOG_ERROR, "Cannot identify new stream\n"); return AVERROR_INVALIDDATA; @@ -430,7 +430,7 @@ static int ogg_read_page(AVFormatContext *s, int *sid, int probing) /* CRC is correct so we can be 99% sure there's an actual change here */ if (idx < 0) { if (data_packets_seen(ogg)) - idx = ogg_replace_stream(s, serial, readout_buf, probing); + idx = ogg_replace_stream(s, serial, readout_buf, size, probing); else idx = ogg_new_stream(s, serial);
Fixes: regression since e983197cbc93420b67aa7e811be47d7278c2c8a2 Fixes: out of array read Fixes: 22185/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5662069073641472 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/oggdec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)