[FFmpeg-devel] [PATCH] avformat/mov: check offset for overflow in mov_probe()

[James Almer]

On 4/4/2021 6:44 PM, Michael Niedermayer wrote:
> Fixes: Invalid read of size 4
> Fixes: ASAN_Deadlysignal.zip
> 
> Found-by: Hardik Shah <hardik05 at gmail.com>
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>   libavformat/mov.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 7805330bf9..ef73f3199d 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7161,6 +7161,8 @@ static int mov_probe(const AVProbeData *p)
>               score  = FFMAX(score, AVPROBE_SCORE_EXTENSION);
>               break;
>           }
> +        if ((uint64_t)size + 8 > INT64_MAX - offset)
> +            break;
>           offset += size;
>       }
>       if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {

Would something like this also work?

> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 1974498d1e..cd9d9996b3 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -7119,7 +7119,7 @@ static int mov_probe(const AVProbeData *p)
>          int64_t size;
>          int minsize = 8;
>          /* ignore invalid offset */
> -        if ((offset + 8) > (unsigned int)p->buf_size)
> +        if ((offset + 8ULL) > (unsigned int)p->buf_size)
>              break;
>          size = AV_RB32(p->buf + offset);
>          if (size == 1 && offset + 16 <= (unsigned int)p->buf_size) {
> @@ -7166,6 +7166,8 @@ static int mov_probe(const AVProbeData *p)
>              score  = FFMAX(score, AVPROBE_SCORE_EXTENSION);
>              break;
>          }
> +        if (size > INT64_MAX - offset)
> +            break;
>          offset += size;
>      }
>      if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {

I think it conveys what it's trying to do more clearly than your 
version, where the + 8 on top of size is not immediately clear where it 
comes from.