| Message ID | 20211222124728.7300-1-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/makex86 | warning | New warnings during build |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
| andriy/makeppc | warning | New warnings during build |
Gyan Doshi (12021-12-22): > Helper function to check if stored box size is correct and looks > to be fully available. > --- > libavformat/mov.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 2aed6e80ef..7de95b7ab0 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -80,6 +80,40 @@ static int mov_read_mfra(MOVContext *c, AVIOContext *f); > static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size, > int count, int duration); > > +/** Check if the box size meets the requirements passed in limit and constraint_type. > + * If input avio_size is valid, it checks if box size appears to be available. > + * > + * constraint_type may be > + * 0 if the box size has to be exactly equal to limit > + * -1 if the box size has to be at most limit > + * 1 if the box size has to be at least limit > + * > + * Returns 0 if size meets requirements. > + */ > +static int validate_box_size(MOVContext *c, MOVAtom atom, AVIOContext *pb, > + int64_t pos, int64_t limit, int constraint_type) > +{ > + int size_fit; > + int64_t input_size = avio_size(pb); > + > + if (input_size > 0 && > + input_size - pos < atom.size) { > + av_log(c->fc, AV_LOG_ERROR, "Box %s is truncated\n", av_fourcc2str(atom.type)); > + return AVERROR_INVALIDDATA; > + } > + > + if (FFABS(constraint_type) > 1) > + return AVERROR_BUG; av_assert() whould have been the right choice here. > + > + switch(constraint_type) { > + case 0: size_fit = atom.size == limit; break; > + case -1: size_fit = atom.size <= limit; break; > + case 1: size_fit = atom.size >= limit; break; This code is unused, AFAICS. Not a good idea. > + } > + > + return !size_fit; > +} > + > static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb, > unsigned len, const char *key) > { I think the changes belong in a single patch. Regards,
On 2021-12-22 06:43 pm, Nicolas George wrote: > Gyan Doshi (12021-12-22): >> Helper function to check if stored box size is correct and looks >> to be fully available. >> --- >> libavformat/mov.c | 34 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 34 insertions(+) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 2aed6e80ef..7de95b7ab0 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -80,6 +80,40 @@ static int mov_read_mfra(MOVContext *c, AVIOContext *f); >> static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size, >> int count, int duration); >> >> +/** Check if the box size meets the requirements passed in limit and constraint_type. >> + * If input avio_size is valid, it checks if box size appears to be available. >> + * >> + * constraint_type may be >> + * 0 if the box size has to be exactly equal to limit >> + * -1 if the box size has to be at most limit >> + * 1 if the box size has to be at least limit >> + * >> + * Returns 0 if size meets requirements. >> + */ >> +static int validate_box_size(MOVContext *c, MOVAtom atom, AVIOContext *pb, >> + int64_t pos, int64_t limit, int constraint_type) >> +{ >> + int size_fit; >> + int64_t input_size = avio_size(pb); >> + >> + if (input_size > 0 && >> + input_size - pos < atom.size) { >> + av_log(c->fc, AV_LOG_ERROR, "Box %s is truncated\n", av_fourcc2str(atom.type)); >> + return AVERROR_INVALIDDATA; >> + } >> + >> + if (FFABS(constraint_type) > 1) >> + return AVERROR_BUG; > av_assert() whould have been the right choice here. Will change. > >> + >> + switch(constraint_type) { >> + case 0: size_fit = atom.size == limit; break; >> + case -1: size_fit = atom.size <= limit; break; >> + case 1: size_fit = atom.size >= limit; break; > This code is unused, AFAICS. Not a good idea. I'll call this function in other box readers. Their requirements are different. That's why I made the check a separate function. Regards, Gyan
diff --git a/libavformat/mov.c b/libavformat/mov.c index 2aed6e80ef..7de95b7ab0 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -80,6 +80,40 @@ static int mov_read_mfra(MOVContext *c, AVIOContext *f); static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size, int count, int duration); +/** Check if the box size meets the requirements passed in limit and constraint_type. + * If input avio_size is valid, it checks if box size appears to be available. + * + * constraint_type may be + * 0 if the box size has to be exactly equal to limit + * -1 if the box size has to be at most limit + * 1 if the box size has to be at least limit + * + * Returns 0 if size meets requirements. + */ +static int validate_box_size(MOVContext *c, MOVAtom atom, AVIOContext *pb, + int64_t pos, int64_t limit, int constraint_type) +{ + int size_fit; + int64_t input_size = avio_size(pb); + + if (input_size > 0 && + input_size - pos < atom.size) { + av_log(c->fc, AV_LOG_ERROR, "Box %s is truncated\n", av_fourcc2str(atom.type)); + return AVERROR_INVALIDDATA; + } + + if (FFABS(constraint_type) > 1) + return AVERROR_BUG; + + switch(constraint_type) { + case 0: size_fit = atom.size == limit; break; + case -1: size_fit = atom.size <= limit; break; + case 1: size_fit = atom.size >= limit; break; + } + + return !size_fit; +} + static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb, unsigned len, const char *key) {