diff mbox

[FFmpeg-devel] lavf/mov.c: Avoid heap allocation wraps and OOB in mov_read_{senc, saiz, udta_string}()

Message ID CAADho6PaCoj7WWxYmTrua1eSk=xennSdLK0YGnGC2pLiE2Wdyg@mail.gmail.com
State Superseded
Headers show

Commit Message

Matthew Wolenetz Dec. 14, 2016, 11:37 p.m. UTC 
Core of patch is from paul@paulmehta.com
Reference https://crbug.com/643952

Comments

Andreas Cadhalpun Dec. 15, 2016, 1:40 a.m. UTC | #1 
On 15.12.2016 00:37, Matthew Wolenetz wrote:
> From 8622f9398e7c89a664c4c2ceff9d35b89ff17bb5 Mon Sep 17 00:00:00 2001
> From: Matt Wolenetz <wolenetz@chromium.org>
> Date: Tue, 6 Dec 2016 12:54:23 -0800
> Subject: [PATCH] lavf/mov.c: Avoid heap allocation wraps and OOB in
>  mov_read_{senc,saiz,udta_string}()
> 
> Core of patch is from paul@paulmehta.com
> Reference https://crbug.com/643952
> ---
>  libavformat/mov.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index e506d20..87ad91a 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -404,7 +404,7 @@ retry:
>                  return ret;
>              } else if (!key && c->found_hdlr_mdta && c->meta_keys) {
>                  uint32_t index = AV_RB32(&atom.type);
> -                if (index < c->meta_keys_count) {
> +                if (index < c->meta_keys_count && index > 0) {

This should be in a separate patch.

>                      key = c->meta_keys[index];
>                  } else {
>                      av_log(c->fc, AV_LOG_WARNING,
> @@ -4502,8 +4502,8 @@ static int mov_read_senc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  
>      avio_rb32(pb);        /* entries */
>  
> -    if (atom.size < 8) {
> -        av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too small\n", atom.size);
> +    if (atom.size < 8 || atom.size > UINT_MAX) {
> +        av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" invalid\n", atom.size);
>          return AVERROR_INVALIDDATA;
>      }
>  
> @@ -4571,6 +4571,11 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          return 0;
>      }
>  
> +    if (atom.size > UINT_MAX) {
> +        av_log(c->fc, AV_LOG_ERROR, "saiz atom auxiliary_info_sizes size %"PRId64" invalid\n", atom.size);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
>      /* save the auxiliary info sizes as is */
>      data_size = atom.size - atom_header_size;
>  

And these should also check for SIZE_MAX.

Best regards,
Andreas
diff mbox

Patch

From 8622f9398e7c89a664c4c2ceff9d35b89ff17bb5 Mon Sep 17 00:00:00 2001
From: Matt Wolenetz <wolenetz@chromium.org>
Date: Tue, 6 Dec 2016 12:54:23 -0800
Subject: [PATCH] lavf/mov.c: Avoid heap allocation wraps and OOB in
 mov_read_{senc,saiz,udta_string}()

Core of patch is from paul@paulmehta.com
Reference https://crbug.com/643952
---
 libavformat/mov.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index e506d20..87ad91a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -404,7 +404,7 @@  retry:
                 return ret;
             } else if (!key && c->found_hdlr_mdta && c->meta_keys) {
                 uint32_t index = AV_RB32(&atom.type);
-                if (index < c->meta_keys_count) {
+                if (index < c->meta_keys_count && index > 0) {
                     key = c->meta_keys[index];
                 } else {
                     av_log(c->fc, AV_LOG_WARNING,
@@ -4502,8 +4502,8 @@  static int mov_read_senc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     avio_rb32(pb);        /* entries */
 
-    if (atom.size < 8) {
-        av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" too small\n", atom.size);
+    if (atom.size < 8 || atom.size > UINT_MAX) {
+        av_log(c->fc, AV_LOG_ERROR, "senc atom size %"PRId64" invalid\n", atom.size);
         return AVERROR_INVALIDDATA;
     }
 
@@ -4571,6 +4571,11 @@  static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return 0;
     }
 
+    if (atom.size > UINT_MAX) {
+        av_log(c->fc, AV_LOG_ERROR, "saiz atom auxiliary_info_sizes size %"PRId64" invalid\n", atom.size);
+        return AVERROR_INVALIDDATA;
+    }
+
     /* save the auxiliary info sizes as is */
     data_size = atom.size - atom_header_size;
 
-- 
2.8.0.rc3.226.g39d4020