| Message ID | 20211222124728.7300-2-ffmpeg@gyani.pro |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/2] avformat/mov: add validate_box_size | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Wed, Dec 22, 2021 at 06:17:28PM +0530, Gyan Doshi wrote: > --- > libavformat/mov.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 7de95b7ab0..1e44c74944 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -2969,6 +2969,12 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) > avio_rb24(pb); /* flags */ > entries = avio_rb32(pb); > > + if (validate_box_size(c, atom, pb, avio_tell(pb)-8, 8+(int64_t)entries*8, 0)) { > + av_log(c->fc, AV_LOG_ERROR, "Invalid or incomplete %s box in stream %d\n", > + av_fourcc2str(atom.type), c->fc->nb_streams-1); > + return AVERROR_INVALIDDATA; > + } > + > av_log(c->fc, AV_LOG_TRACE, "track[%u].stts.entries = %u\n", > c->fc->nb_streams-1, entries); > this breaks playback of ./ffplay H263_NM_f.mp4 i presume its this file here: https://samples.ffmpeg.org/F4V/H263_NM_f.mp4 thx [...]
On 2021-12-23 03:04 am, Michael Niedermayer wrote: > On Wed, Dec 22, 2021 at 06:17:28PM +0530, Gyan Doshi wrote: >> --- >> libavformat/mov.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 7de95b7ab0..1e44c74944 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -2969,6 +2969,12 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >> avio_rb24(pb); /* flags */ >> entries = avio_rb32(pb); >> >> + if (validate_box_size(c, atom, pb, avio_tell(pb)-8, 8+(int64_t)entries*8, 0)) { >> + av_log(c->fc, AV_LOG_ERROR, "Invalid or incomplete %s box in stream %d\n", >> + av_fourcc2str(atom.type), c->fc->nb_streams-1); >> + return AVERROR_INVALIDDATA; >> + } >> + >> av_log(c->fc, AV_LOG_TRACE, "track[%u].stts.entries = %u\n", >> c->fc->nb_streams-1, entries); >> > this breaks playback of > > ./ffplay H263_NM_f.mp4 Sent revised set. However, do we need to allow this? The file has multiple invalid boxes. gpac refuses to import it. vlc using its default mp4 demuxer does not open it, neither does wmp or firefox. Seems only avformat users can open the file. Regards, Gyan
On Thu, Dec 23, 2021 at 12:54:58PM +0530, Gyan Doshi wrote: > > > On 2021-12-23 03:04 am, Michael Niedermayer wrote: > > On Wed, Dec 22, 2021 at 06:17:28PM +0530, Gyan Doshi wrote: > > > --- > > > libavformat/mov.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > > index 7de95b7ab0..1e44c74944 100644 > > > --- a/libavformat/mov.c > > > +++ b/libavformat/mov.c > > > @@ -2969,6 +2969,12 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > > avio_rb24(pb); /* flags */ > > > entries = avio_rb32(pb); > > > + if (validate_box_size(c, atom, pb, avio_tell(pb)-8, 8+(int64_t)entries*8, 0)) { > > > + av_log(c->fc, AV_LOG_ERROR, "Invalid or incomplete %s box in stream %d\n", > > > + av_fourcc2str(atom.type), c->fc->nb_streams-1); > > > + return AVERROR_INVALIDDATA; > > > + } > > > + > > > av_log(c->fc, AV_LOG_TRACE, "track[%u].stts.entries = %u\n", > > > c->fc->nb_streams-1, entries); > > this breaks playback of > > > > ./ffplay H263_NM_f.mp4 > > Sent revised set. > > However, do we need to allow this? yes its a real world file its not crafted to be funky thx [...]
diff --git a/libavformat/mov.c b/libavformat/mov.c index 7de95b7ab0..1e44c74944 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -2969,6 +2969,12 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) avio_rb24(pb); /* flags */ entries = avio_rb32(pb); + if (validate_box_size(c, atom, pb, avio_tell(pb)-8, 8+(int64_t)entries*8, 0)) { + av_log(c->fc, AV_LOG_ERROR, "Invalid or incomplete %s box in stream %d\n", + av_fourcc2str(atom.type), c->fc->nb_streams-1); + return AVERROR_INVALIDDATA; + } + av_log(c->fc, AV_LOG_TRACE, "track[%u].stts.entries = %u\n", c->fc->nb_streams-1, entries);