Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Am 17.10.14 17:03, schrieb Michael Niedermayer:
[...]
i think it would be simpler to always allocate with no str_small
local buffer case.
Also always allocating 2x the size should avoid the macro
Done attached.
-Thilo
From e374fd065392f7e68fd3d951712a13008dc6fb2f Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@mail.de
Date: Mon, 20 Oct 2014 13:42:28 +0200
Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
---
libavformat/mov.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 4ff46dd..fee76e7 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -261,10 +261,11 @@ 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 key2[16], language[4] = {0};
+char *str = NULL;
const char *key = NULL;
uint16_t langcode = 0;
-uint32_t data_type = 0, str_size;
+uint32_t data_type = 0, str_size, str_size_alloc;
int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
switch (atom.type) {
@@ -353,18 +354,21 @@ static int mov_read_udta_string(MOVContext *c,
AVIOContext *pb, MOVAtom atom)
}
#endif
+str_size_alloc = str_size 1; // worst-case requirement for output
string in case of utf8 coded input
+str = av_malloc(str_size_alloc);
+if (!str)
+return AVERROR(ENOMEM);
+
if (!key)
return 0;
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 (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_alloc);
} else {
int ret = avio_read(pb, str, str_size);
if (ret != str_size)
@@ -380,7 +384,9 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext
*pb, MOVAtom atom)
}
av_dlog(c-fc, lang \%3s\ , language);
av_dlog(c-fc, tag \%s\ value \%s\ atom \%.4s\ %d %PRId64\n,
-key, str, (char*)atom.type, str_size, atom.size);
+key, str, (char*)atom.type, str_size_alloc, atom.size);
+
+av_freep(str);
return 0;
}
--
1.9.3 (Apple Git-50)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
On Mon, Oct 20, 2014 at 01:43:32PM +0200, Thilo Borgmann wrote: Am 17.10.14 17:03, schrieb Michael Niedermayer: [...] i think it would be simpler to always allocate with no str_small local buffer case. Also always allocating 2x the size should avoid the macro Done attached. -Thilo mov.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) 76e7a087fd36eac892308af02cc8d37c287c142f 0001-lavf-mov.c-Allocate-buffer-in-case-of-long-metadata-.patch From e374fd065392f7e68fd3d951712a13008dc6fb2f Mon Sep 17 00:00:00 2001 From: Thilo Borgmann thilo.borgm...@mail.de Date: Mon, 20 Oct 2014 13:42:28 +0200 Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries. applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
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.borgm...@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
From 1a59272e3d333c784e9f4857cd3aa6542ad28d6d Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@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;
}
--
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
On Fri, Oct 17, 2014 at 02:33:27PM +0200, Thilo Borgmann wrote:
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.borgm...@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
mov.c | 27 ++-
1 file changed, 22 insertions(+), 5 deletions(-)
c6706cd53f0c804d993ba5dec8112faf1b9e84e5
0001-lavf-mov.c-Allocate-buffer-in-case-of-long-metadata-.patch
From 1a59272e3d333c784e9f4857cd3aa6542ad28d6d Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@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);
i
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Am 13.10.14 13:08, schrieb Nicolas George: Le duodi 22 vendémiaire, an CCXXIII, Reimar Döffinger a écrit : Uh, wouldn't that be undefined behaviour? You're not allowed to compare pointers from different allocations as far as I know, even if it usually works. The spec says: # Two pointers compare equal if and only if both are null pointers, both are # pointers to the same object (including a pointer to an object and a # subobject at its beginning) or function, both are pointers to one past the # last element of the same array object, or one is a pointer to one past the # end of one array object and the other is a pointer to the start of a # different array object that happens to immediately follow the first array # object in the address space.) If I read this correctly, the and only if means that comparing pointers from different allocations is valid and returns false. I suspect you are generalising the issue with and : for these operators, comparing pointers to different objects is invalid. Ping. The pointer issue does not apply to the current patch. -Thilo ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
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.borgm...@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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Am 11.10.14 16:19, schrieb Nicolas George:
[...]
all remarks applied.
-Thilo
From 5a14ef97ffc7d82dea5644c736e6dc2de2079e89 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@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);
} else {
int ret = avio_read(pb, str, str_size);
if (ret != str_size)
@@ -382,6 +388,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)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Am 13.10.14 09:57, schrieb Reimar Döffinger:
On 11.10.2014, at 16:19, Nicolas George geo...@nsup.org wrote:
+if (str_size sizeof(str)-1) { // free buffer for long data field
+av_freep(pstr);
I think if (pstr != str) would be more robust.
Uh, wouldn't that be undefined behaviour?
You're not allowed to compare pointers from different allocations as far as I
know, even if it usually works.
The original comparison is still in the patch and no pointers are compared.
Which does not mean I'm sure about the comparing issue you mention.
-Thilo
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Le duodi 22 vendémiaire, an CCXXIII, Reimar Döffinger a écrit : Uh, wouldn't that be undefined behaviour? You're not allowed to compare pointers from different allocations as far as I know, even if it usually works. The spec says: # Two pointers compare equal if and only if both are null pointers, both are # pointers to the same object (including a pointer to an object and a # subobject at its beginning) or function, both are pointers to one past the # last element of the same array object, or one is a pointer to one past the # end of one array object and the other is a pointer to the start of a # different array object that happens to immediately follow the first array # object in the address space.) If I read this correctly, the and only if means that comparing pointers from different allocations is valid and returns false. I suspect you are generalising the issue with and : for these operators, comparing pointers to different objects is invalid. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Hi,
trying to fix ticket #4018.
Metadata in mov is silently truncated to 1023 bytes.
This patch allocated a buffer in case of entries found that exceed 1023 bytes.
Fixes ticket #4018 for me.
Maybe check str_size against an upper limit?
-Thilo
From 365bec36b3b7f1925cfa2310d979a63ef8e3a7e8 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@mail.de
Date: Sat, 11 Oct 2014 16:09:07 +0200
Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
---
libavformat/mov.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 4ff46dd..136b1d5 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -358,24 +358,33 @@ 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 {
+char *pstr = str;
+if (str_size sizeof(str)-1) { // allocate buffer for long data field
+pstr = 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, pstr, str_size);
} else {
-int ret = avio_read(pb, str, str_size);
+int ret = avio_read(pb, pstr, str_size);
if (ret != str_size)
return ret 0 ? ret : AVERROR_INVALIDDATA;
-str[str_size] = 0;
+pstr[str_size] = 0;
}
c-fc-event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
-av_dict_set(c-fc-metadata, key, str, 0);
+av_dict_set(c-fc-metadata, key, pstr, 0);
if (*language strcmp(language, und)) {
snprintf(key2, sizeof(key2), %s-%s, key, language);
-av_dict_set(c-fc-metadata, key2, str, 0);
+av_dict_set(c-fc-metadata, key2, pstr, 0);
+}
+
+if (str_size sizeof(str)-1) { // free buffer for long data field
+av_freep(pstr);
}
}
av_dlog(c-fc, lang \%3s\ , language);
--
1.9.3 (Apple Git-50)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
Le decadi 20 vendémiaire, an CCXXIII, Thilo Borgmann a écrit :
Hi,
trying to fix ticket #4018.
Metadata in mov is silently truncated to 1023 bytes.
This patch allocated a buffer in case of entries found that exceed 1023 bytes.
Fixes ticket #4018 for me.
Maybe check str_size against an upper limit?
-Thilo
From 365bec36b3b7f1925cfa2310d979a63ef8e3a7e8 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann thilo.borgm...@mail.de
Date: Sat, 11 Oct 2014 16:09:07 +0200
Subject: [PATCH] lavf/mov.c: Allocate buffer in case of long metadata entries.
---
libavformat/mov.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 4ff46dd..136b1d5 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -358,24 +358,33 @@ 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 {
+char *pstr = str;
If you rename str (maybe str_small) and call pstr str instead, I believe
that makes the patch simpler.
+if (str_size sizeof(str)-1) { // allocate buffer for long data
field
+pstr = 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, pstr, str_size);
} else {
-int ret = avio_read(pb, str, str_size);
+int ret = avio_read(pb, pstr, str_size);
if (ret != str_size)
return ret 0 ? ret : AVERROR_INVALIDDATA;
-str[str_size] = 0;
+pstr[str_size] = 0;
}
c-fc-event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
-av_dict_set(c-fc-metadata, key, str, 0);
+av_dict_set(c-fc-metadata, key, pstr, 0);
if (*language strcmp(language, und)) {
snprintf(key2, sizeof(key2), %s-%s, key, language);
-av_dict_set(c-fc-metadata, key2, str, 0);
+av_dict_set(c-fc-metadata, key2, pstr, 0);
+}
+
+if (str_size sizeof(str)-1) { // free buffer for long data field
+av_freep(pstr);
I think if (pstr != str) would be more robust.
Even more robust, if a bit more verbose:
char *pstr = str, *alloc_str = NULL;
if (...) {
pstr = alloc_str = malloc(...);
}
...
av_freep(alloc_str); // unconditionnal
}
}
av_dlog(c-fc, lang \%3s\ , language);
Regards,
--
Nicolas George
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
