[FFmpeg-devel,2/8] avformat/mov: Support size = 1 and size = 0 special cases in probing

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/mov.c | 5 +++++
 1 file changed, 5 insertions(+)

On 2/6/2021 9:22 AM, Michael Niedermayer wrote:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavformat/mov.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 9406e42f49..70f76caff5 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7113,6 +7113,11 @@ static int mov_probe(const AVProbeData *p)
>           if ((offset + 8) > (unsigned int)p->buf_size)
>               break;
>           size = AV_RB32(p->buf + offset);
> +        if (size == 1 && offset + 16 > (unsigned int)p->buf_size) {
> +            size = AV_RB64(p->buf+offset + 8);

Just curious, what happens when size == 1 and the buffer is too small? 
Is leaving it as a size of 1 still valid, or should it be handled as a 
format error (e.g. abort the loop)?


> +        } else if (size == 0) {
> +            size = p->buf_size - offset;
> +        }
>           tag = AV_RL32(p->buf + offset + 4);
>           switch(tag) {
>           /* check for obvious tags */
>

On Sat, Feb 06, 2021 at 11:33:38AM -0800, Chad Fraleigh wrote:
> On 2/6/2021 9:22 AM, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavformat/mov.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 9406e42f49..70f76caff5 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7113,6 +7113,11 @@ static int mov_probe(const AVProbeData *p)
> >           if ((offset + 8) > (unsigned int)p->buf_size)
> >               break;
> >           size = AV_RB32(p->buf + offset);
> > +        if (size == 1 && offset + 16 > (unsigned int)p->buf_size) {
> > +            size = AV_RB64(p->buf+offset + 8);
> 
> Just curious, what happens when size == 1 and the buffer is too small? Is
> leaving it as a size of 1 still valid, or should it be handled as a format
> error (e.g. abort the loop)?

The buffer must have a minimum padding of AVPROBE_PADDING_SIZE
so the buffer cannot be too small. This extra padding requirement is
there for exactly cases like this, otherwise alot more checks would be
needed in many probe functions

thx

[...]

On Mon, Feb 08, 2021 at 02:25:50PM +0100, Michael Niedermayer wrote:
> On Sat, Feb 06, 2021 at 11:33:38AM -0800, Chad Fraleigh wrote:
> > On 2/6/2021 9:22 AM, Michael Niedermayer wrote:
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > >   libavformat/mov.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > > index 9406e42f49..70f76caff5 100644
> > > --- a/libavformat/mov.c
> > > +++ b/libavformat/mov.c
> > > @@ -7113,6 +7113,11 @@ static int mov_probe(const AVProbeData *p)
> > >           if ((offset + 8) > (unsigned int)p->buf_size)
> > >               break;
> > >           size = AV_RB32(p->buf + offset);
> > > +        if (size == 1 && offset + 16 > (unsigned int)p->buf_size) {
> > > +            size = AV_RB64(p->buf+offset + 8);
> > 
> > Just curious, what happens when size == 1 and the buffer is too small? Is
> > leaving it as a size of 1 still valid, or should it be handled as a format
> > error (e.g. abort the loop)?
> 
> The buffer must have a minimum padding of AVPROBE_PADDING_SIZE
> so the buffer cannot be too small. This extra padding requirement is
> there for exactly cases like this, otherwise alot more checks would be
> needed in many probe functions

will apply

[...]