[FFmpeg-devel] [PATCH] mov demuxer: Check if a key is longer than the atom containing it

[James Almer]

On 4/1/2024 8:12 PM, Eugene Zemtsov via ffmpeg-devel wrote:
> From: Eugene Zemtsov <eugene at chromium.org>
> 
> Stop reading keys and return AVERROR_INVALIDDATA if key_size
> is larger than the amount of space left in the atom.
> 
> Bug: https://crbug.com/41496983
> Signed-off-by: Eugene Zemtsov <eugene at chromium.org>
> ---
>   libavformat/mov.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 662301bf67..5d2f7fa690 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5045,15 +5045,18 @@ static int mov_read_keys(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>       if (!c->meta_keys)
>           return AVERROR(ENOMEM);
>   
> +    uint32_t bytes_left_in_atom = atom.size;
>       for (i = 1; i <= count; ++i) {
>           uint32_t key_size = avio_rb32(pb);
>           uint32_t type = avio_rl32(pb);
> -        if (key_size < 8) {
> +        if (key_size < 8 || key_size > bytes_left_in_atom) {
>               av_log(c->fc, AV_LOG_ERROR,
>                      "The key# %"PRIu32" in meta has invalid size:"
>                      "%"PRIu32"\n", i, key_size);
>               return AVERROR_INVALIDDATA;
>           }
> +
> +        bytes_left_in_atom -= key_size;

atom is a copy and not used anywhere else in this function, so you can 
just do atom.size -= key_size and check for that above instead.

>           key_size -= 8;
>           if (type != MKTAG('m','d','t','a')) {
>               avio_skip(pb, key_size);