[FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.

[Thilo Borgmann]

Am 16.10.14 04:47, schrieb Michael Niedermayer:
> On Mon, Oct 13, 2014 at 09:40:42AM +0200, Thilo Borgmann wrote:
>> Am 11.10.14 16:19, schrieb Nicolas George:
>>> [...]
>>
>> all remarks applied.
>>
>> -Thilo
>>
> 
>>  mov.c |   16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>> cabb6e51de7f9329603561773f209b6a948478ce  0001-lavf-mov.c-Allocate-buffer-in-case-of-long-metadata-.patch
>> From 5a14ef97ffc7d82dea5644c736e6dc2de2079e89 Mon Sep 17 00:00:00 2001
>> From: Thilo Borgmann <thilo.borgmann at mail.de>
>> Date: Mon, 13 Oct 2014 09:36:17 +0200
>> Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
>>
>> ---
>>  libavformat/mov.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 4ff46dd..8d6d074 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -261,7 +261,9 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>  #ifdef MOV_EXPORT_ALL_METADATA
>>      char tmp_key[5];
>>  #endif
>> -    char str[1024], key2[16], language[4] = {0};
>> +    char str_small[1024], key2[16], language[4] = {0};
>> +    char *str = str_small;
>> +    char *pstr = NULL;
>>      const char *key = NULL;
>>      uint16_t langcode = 0;
>>      uint32_t data_type = 0, str_size;
>> @@ -358,13 +360,17 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>      if (atom.size < 0)
>>          return AVERROR_INVALIDDATA;
>>  
>> -    str_size = FFMIN3(sizeof(str)-1, str_size, atom.size);
>> -
>>      if (parse)
>>          parse(c, pb, str_size, key);
>>      else {
>> +        if (str_size > sizeof(str_small)-1) { // allocate buffer for long data field
>> +            pstr = str = av_malloc(str_size);
>> +            if (!pstr)
>> +                return AVERROR(ENOMEM);
>> +        }
>> +
>>          if (data_type == 3 || (data_type == 0 && (langcode < 0x400 || langcode == 0x7fff))) { // MAC Encoded
>> -            mov_read_mac_string(c, pb, str_size, str, sizeof(str));
>> +            mov_read_mac_string(c, pb, str_size, str, str_size);
> 
> this seems to store UTF8, which can require more space than str_size

New patch attached using a worst-case size of twice the input string size if
the input is in utf8.

Tested only with non utf8 by now - I would appreciate it if someone could test
this with UTF8 metadata or tell me how to generate/where to get a suitable file.

-Thilo

-------------- next part --------------
>From 1a59272e3d333c784e9f4857cd3aa6542ad28d6d Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Fri, 17 Oct 2014 14:30:30 +0200
Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.

---
 libavformat/mov.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 4ff46dd..a48877d 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -261,7 +261,9 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 #ifdef MOV_EXPORT_ALL_METADATA
     char tmp_key[5];
 #endif
-    char str[1024], key2[16], language[4] = {0};
+    char str_small[1024], key2[16], language[4] = {0};
+    char *str = str_small;
+    char *pstr = NULL;
     const char *key = NULL;
     uint16_t langcode = 0;
     uint32_t data_type = 0, str_size;
@@ -358,15 +360,28 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (atom.size < 0)
         return AVERROR_INVALIDDATA;
 
-    str_size = FFMIN3(sizeof(str)-1, str_size, atom.size);
-
     if (parse)
         parse(c, pb, str_size, key);
     else {
+#define LONG_META_ALLOC() {                   \
+        if (str_size > sizeof(str_small)-1) { \
+            pstr = str = av_malloc(str_size); \
+            if (!pstr)                        \
+                return AVERROR(ENOMEM);       \
+        }                                     \
+}
+
         if (data_type == 3 || (data_type == 0 && (langcode < 0x400 || langcode == 0x7fff))) { // MAC Encoded
-            mov_read_mac_string(c, pb, str_size, str, sizeof(str));
+            int str_size_in = str_size;
+            str_size <<= 1; // worst-case requirement for output string in case of utf8 coded input
+            // allocate buffer for long data field if necessary
+            LONG_META_ALLOC();
+            mov_read_mac_string(c, pb, str_size_in, str, str_size);
         } else {
-            int ret = avio_read(pb, str, str_size);
+            int ret;
+            // allocate buffer for long data field if necessary
+            LONG_META_ALLOC();
+            ret = avio_read(pb, str, str_size);
             if (ret != str_size)
                 return ret < 0 ? ret : AVERROR_INVALIDDATA;
             str[str_size] = 0;
@@ -382,6 +397,8 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     av_dlog(c->fc, "tag \"%s\" value \"%s\" atom \"%.4s\" %d %"PRId64"\n",
             key, str, (char*)&atom.type, str_size, atom.size);
 
+    av_freep(&pstr);
+
     return 0;
 }
 
-- 
1.9.3 (Apple Git-50)