diff mbox

[FFmpeg-devel] lavf/concatdec: Allocate the filenames on the heap

Message ID CAB0OVGoNJ1ZTYmpUPCQ+S9BWjpc-TPDFrkj+wpmDxamjop2V-A@mail.gmail.com
State Superseded
Headers show

Commit Message

Carl Eugen Hoyos Oct. 20, 2017, 8:22 p.m. UTC 
Hi!

I believe that the use-case is valid and there is definitely a bug
because no error is shown for too long filenames.
In addition, allocating 50k on the heap seems nicer than 4k on the stack.

Please comment, Carl Eugen

Comments

Marton Balint Oct. 21, 2017, 4:22 p.m. UTC | #1 
On Fri, 20 Oct 2017, Carl Eugen Hoyos wrote:

> Hi!
>
> I believe that the use-case is valid and there is definitely a bug
> because no error is shown for too long filenames.
> In addition, allocating 50k on the heap seems nicer than 4k on the stack.

50k is not much better than 4k. You should add an ff_get_line variant 
which dynamically allocates the needed buffer.

Also you should free buf somewhere eventually.

I thought filenames in libavformat are limited to 1K anyway because of the 
AVFormatContext->filename field. So if your patch really fixes the ticket, 
then how? :)

Thanks,
Marton
Nicolas George Oct. 21, 2017, 4:29 p.m. UTC | #2 
Le decadi 30 vendémiaire, an CCXXVI, Marton Balint a écrit :
> I thought filenames in libavformat are limited to 1K anyway because of the
> AVFormatContext->filename field. So if your patch really fixes the ticket,
> then how? :)

Yes, exactly. This limitation was the reason I did not bother handling
longer lines. I would like to understand how it makes a difference.

Regards,
diff mbox

Patch

From e25b8e6b77e3b9b661f1ce00beeef7e057929ba3 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Fri, 20 Oct 2017 22:19:00 +0200
Subject: [PATCH] lavf/concatdec: Allocate the filenames on the heap.

Allow huge filenames, real-world content can be passed via data: protocol.

Fixes ticket #6761.
---
 libavformat/concatdec.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 0e18901..739feaf 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -386,16 +386,24 @@  static int concat_read_close(AVFormatContext *avf)
 static int concat_read_header(AVFormatContext *avf)
 {
     ConcatContext *cat = avf->priv_data;
-    uint8_t buf[4096];
-    uint8_t *cursor, *keyword;
+    int buf_size = 50002;
+    uint8_t *buf, *cursor, *keyword;
     int ret, line = 0, i;
     unsigned nb_files_alloc = 0;
     ConcatFile *file = NULL;
     int64_t time = 0;
 
+    buf = av_malloc(buf_size);
+    if (!buf)
+        return AVERROR(ENOMEM);
+
     while (1) {
-        if ((ret = ff_get_line(avf->pb, buf, sizeof(buf))) <= 0)
+        if ((ret = ff_get_line(avf->pb, buf, buf_size)) <= 0)
             break;
+        if (ret == buf_size - 1) {
+            av_log(avf, AV_LOG_ERROR, "filename too long (>%d)\n", buf_size - 2);
+            FAIL(AVERROR_INVALIDDATA);
+        }
         line++;
         cursor = buf;
         keyword = get_keyword(&cursor);
-- 
1.7.10.4