Re: [libav-devel] [PATCH 1/5] mov: Fix spherical metadata_source parsing

[Vittorio Giovara]

On Wed, Feb 15, 2017 at 2:22 AM, Anton Khirnov  wrote:
> Quoting Vittorio Giovara (2017-02-10 22:08:07)
>> From: Aaron Colwell 
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavformat/mov.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 2810960..4a6f9c0 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -3255,7 +3255,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
>> *pb, MOVAtom atom)
>>  return 0;
>>  }
>>  avio_skip(pb, 4); /*  version + flags */
>> -avio_skip(pb, avio_r8(pb)); /* metadata_source */
>> +avio_skip(pb, size - 12); /* metadata_source */
>>
>>  size = avio_rb32(pb);
>>  if (size > atom.size)
>> @@ -3268,7 +3268,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
>> *pb, MOVAtom atom)
>>  }
>>
>>  size = avio_rb32(pb);
>> -if (size > atom.size)
>> +if (size <= 12 || size > atom.size)
>>  return AVERROR_INVALIDDATA;
>>
>>  tag = avio_rl32(pb);
>> --
>> 2.10.0
>
> The first hunk looks ok, but the second one is strange? Why specifically
> that check. I see a bunch of similar code in this function where similar
> checks might also make sense, yet one is added only here.

hm, yes, the < 12 check should be *before* the first chunk, i guess
git am got confused to which block of code the diff belonged to (and I
didn't check either).
I'll send an updated version, thanks for spotting.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/5] mov: Fix spherical metadata_source parsing

[Anton Khirnov]

Quoting Vittorio Giovara (2017-02-10 22:08:07)
> From: Aaron Colwell 
> 
> Signed-off-by: James Almer 
> ---
>  libavformat/mov.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2810960..4a6f9c0 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3255,7 +3255,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  return 0;
>  }
>  avio_skip(pb, 4); /*  version + flags */
> -avio_skip(pb, avio_r8(pb)); /* metadata_source */
> +avio_skip(pb, size - 12); /* metadata_source */
>  
>  size = avio_rb32(pb);
>  if (size > atom.size)
> @@ -3268,7 +3268,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>  }
>  
>  size = avio_rb32(pb);
> -if (size > atom.size)
> +if (size <= 12 || size > atom.size)
>  return AVERROR_INVALIDDATA;
>  
>  tag = avio_rl32(pb);
> -- 
> 2.10.0

The first hunk looks ok, but the second one is strange? Why specifically
that check. I see a bunch of similar code in this function where similar
checks might also make sense, yet one is added only here.

-- 
Anton Khirnov
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 1/5] mov: Fix spherical metadata_source parsing

[Vittorio Giovara]

From: Aaron Colwell 

Signed-off-by: James Almer 
---
 libavformat/mov.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2810960..4a6f9c0 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3255,7 +3255,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 return 0;
 }
 avio_skip(pb, 4); /*  version + flags */
-avio_skip(pb, avio_r8(pb)); /* metadata_source */
+avio_skip(pb, size - 12); /* metadata_source */
 
 size = avio_rb32(pb);
 if (size > atom.size)
@@ -3268,7 +3268,7 @@ static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 }
 
 size = avio_rb32(pb);
-if (size > atom.size)
+if (size <= 12 || size > atom.size)
 return AVERROR_INVALIDDATA;
 
 tag = avio_rl32(pb);
-- 
2.10.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel