diff mbox

[FFmpeg-devel] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr

Message ID CAADho6Mv96M5rSJw0y_Qf1E-v1dNxR=6KCM2cq5Gca05W8sR6g@mail.gmail.com
State Superseded
Headers show

Commit Message

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

Comments

Andreas Cadhalpun Dec. 15, 2016, 1:39 a.m. UTC | #1
On 15.12.2016 00:34, Matthew Wolenetz wrote:
> 
> From fd878457cd55690d4a27d74411b68a30c9fb2313 Mon Sep 17 00:00:00 2001
> From: Matt Wolenetz <wolenetz@chromium.org>
> Date: Fri, 2 Dec 2016 18:10:39 -0800
> Subject: [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr
> 
> Core of patch is from paul@paulmehta.com
> Reference https://crbug.com/643950
> ---
>  libavformat/mov.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2a69890..7254505 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -739,6 +739,8 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  
>      title_size = atom.size - 24;
>      if (title_size > 0) {
> +        if (title_size >= UINT_MAX)

I think this should use SIZE_MAX.

> +            return AVERROR_INVALIDDATA;
>          title_str = av_malloc(title_size + 1); /* Add null terminator */
>          if (!title_str)
>              return AVERROR(ENOMEM);

Best regards,
Andreas
James Almer Dec. 15, 2016, 2:25 a.m. UTC | #2
On 12/14/2016 10:39 PM, Andreas Cadhalpun wrote:
> On 15.12.2016 00:34, Matthew Wolenetz wrote:
>>
>> From fd878457cd55690d4a27d74411b68a30c9fb2313 Mon Sep 17 00:00:00 2001
>> From: Matt Wolenetz <wolenetz@chromium.org>
>> Date: Fri, 2 Dec 2016 18:10:39 -0800
>> Subject: [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr
>>
>> Core of patch is from paul@paulmehta.com
>> Reference https://crbug.com/643950
>> ---
>>  libavformat/mov.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 2a69890..7254505 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -739,6 +739,8 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>  
>>      title_size = atom.size - 24;
>>      if (title_size > 0) {
>> +        if (title_size >= UINT_MAX)
> 
> I think this should use SIZE_MAX.

title_size is int64_t and SIZE_MAX is UINT64_MAX on x86_64.

> 
>> +            return AVERROR_INVALIDDATA;
>>          title_str = av_malloc(title_size + 1); /* Add null terminator */
>>          if (!title_str)
>>              return AVERROR(ENOMEM);
> 
> Best regards,
> Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Andreas Cadhalpun Dec. 16, 2016, 1:23 a.m. UTC | #3
On 15.12.2016 03:25, James Almer wrote:
> On 12/14/2016 10:39 PM, Andreas Cadhalpun wrote:
>> On 15.12.2016 00:34, Matthew Wolenetz wrote:
>>>
>>> From fd878457cd55690d4a27d74411b68a30c9fb2313 Mon Sep 17 00:00:00 2001
>>> From: Matt Wolenetz <wolenetz@chromium.org>
>>> Date: Fri, 2 Dec 2016 18:10:39 -0800
>>> Subject: [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr
>>>
>>> Core of patch is from paul@paulmehta.com
>>> Reference https://crbug.com/643950
>>> ---
>>>  libavformat/mov.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>>> index 2a69890..7254505 100644
>>> --- a/libavformat/mov.c
>>> +++ b/libavformat/mov.c
>>> @@ -739,6 +739,8 @@ static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>>  
>>>      title_size = atom.size - 24;
>>>      if (title_size > 0) {
>>> +        if (title_size >= UINT_MAX)
>>
>> I think this should use SIZE_MAX.
> 
> title_size is int64_t and SIZE_MAX is UINT64_MAX on x86_64.

Yes, but the argument of av_malloc is size_t.

>>
>>> +            return AVERROR_INVALIDDATA;
>>>          title_str = av_malloc(title_size + 1); /* Add null terminator */

So this should cast the argument to size_t to fix the issue on x86_64:
          title_str = av_malloc((size_t)title_size + 1); /* Add null terminator */

Best regards,
Andreas
diff mbox

Patch

From fd878457cd55690d4a27d74411b68a30c9fb2313 Mon Sep 17 00:00:00 2001
From: Matt Wolenetz <wolenetz@chromium.org>
Date: Fri, 2 Dec 2016 18:10:39 -0800
Subject: [PATCH] lavf/mov.c: Avoid heap allocation wrap in mov_read_hdlr

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

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2a69890..7254505 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -739,6 +739,8 @@  static int mov_read_hdlr(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     title_size = atom.size - 24;
     if (title_size > 0) {
+        if (title_size >= UINT_MAX)
+            return AVERROR_INVALIDDATA;
         title_str = av_malloc(title_size + 1); /* Add null terminator */
         if (!title_str)
             return AVERROR(ENOMEM);
-- 
2.8.0.rc3.226.g39d4020