diff mbox

[FFmpeg-devel] lavf/mov: Fix an out-of-bound-read in mov_read_mac_string().

Message ID 201611141456.04304.cehoyos@ag.or.at
State Rejected
Headers show

Commit Message

Carl Eugen Hoyos Nov. 14, 2016, 1:56 p.m. UTC
Hi!

I believe attached patch fixes an out-of-bound-read in mov_read_mac_string() 
if p<end is false and if the read character is < 0x80, see bug 989.

Please comment, Carl Eugen
From 6c69f755e1c4f22d1efb36777631c98a4d20ffef Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <cehoyos@ag.or.at>
Date: Mon, 14 Nov 2016 14:52:58 +0100
Subject: [PATCH] lavf/mov: Fix an out-of-bound-read in mov_read_mac_string().

---
 libavformat/mov.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Cadhalpun Nov. 14, 2016, 10:26 p.m. UTC | #1
On 14.11.2016 14:56, Carl Eugen Hoyos wrote:
> I believe attached patch fixes an out-of-bound-read in mov_read_mac_string() 
> if p<end is false and if the read character is < 0x80, see bug 989.
> 
> Please comment, Carl Eugen

This patch is not necessary, the issue was fixed with commit 437f5daf0.
If (p < end) is false, the 'else if (p < end)' branch will not be entered.

> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -160,7 +160,7 @@ static int mov_read_mac_string(MOVContext *c, AVIOContext *pb, int len,
>          uint8_t t, c = avio_r8(pb);

However, reusing the variable name of the MOVContext as uint8_t looks strange.

>          if (c < 0x80 && p < end)
>              *p++ = c;
> -        else if (p < end)
> +        else if (c >= 0x80 && p < end)
>              PUT_UTF8(mac_to_unicode[c-0x80], t, if (p < end) *p++ = t;);
>      }
>      *p = 0;
> -- 1.7.10.4

Best regards,
Andreas
Carl Eugen Hoyos Nov. 14, 2016, 11:41 p.m. UTC | #2
2016-11-14 23:26 GMT+01:00 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com>:
> On 14.11.2016 14:56, Carl Eugen Hoyos wrote:
>> I believe attached patch fixes an out-of-bound-read in mov_read_mac_string()
>> if p<end is false and if the read character is < 0x80, see bug 989.
>>
>> Please comment, Carl Eugen
>
> This patch is not necessary, the issue was fixed with commit 437f5daf0.
> If (p < end) is false, the 'else if (p < end)' branch will not be entered.

Sorry for the noise!

>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -160,7 +160,7 @@ static int mov_read_mac_string(MOVContext *c, AVIOContext *pb, int len,
>>          uint8_t t, c = avio_r8(pb);
>
> However, reusing the variable name of the MOVContext as uint8_t looks strange.

Maybe that's what irritated me;-)

Thank you, Carl Eugen
diff mbox

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8d6cc12..21556be 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -160,7 +160,7 @@  static int mov_read_mac_string(MOVContext *c, AVIOContext *pb, int len,
         uint8_t t, c = avio_r8(pb);
         if (c < 0x80 && p < end)
             *p++ = c;
-        else if (p < end)
+        else if (c >= 0x80 && p < end)
             PUT_UTF8(mac_to_unicode[c-0x80], t, if (p < end) *p++ = t;);
     }
     *p = 0;