diff mbox series

[FFmpeg-devel,08/21] avformat/dashdec, hls: Update correct pointer to AVDictionary

Message ID 20200919163610.1099233-8-andreas.rheinhardt@gmail.com
State Accepted
Commit ac2852d7958622322c69d68212d96fe41a595001
Headers show
Series [FFmpeg-devel,01/21] avformat/dashdec: Avoid double free on error
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 19, 2020, 4:35 p.m. UTC
open_url() in the DASH as well in the hls demuxer share a common bug:
They modify an AVDictionary (i.e. set a new entry) given to them as
AVDictionary *, yet if this new entry leads to reallocation and
relocation of the AVDictionary, the caller's pointer will become
dangling, leading to use-after-frees. So pass an AVDictionary **.

(With the current implementation of AVDictionary the above can only
happen if the AVDictionary was empty initially (in which case the
new AVDictionary leaks); furthermore if the I/O is ordinary (i.e. opened
by avio_open2() or ffio_open_whitelist()), the dict is never empty (it
contains an rw_timeout entry from save_avio_options()). So this issue
could only happen if the caller sets a nondefault io_open callback, but
no AVIOContext (the AVFMT_FLAG_CUSTOM_IO flag won't be set in this
case). In case of the HLS demuxer, it was also necessary that setting
the "seekable" entry failed. Yet one should simply not rely on internals
of the AVDict API.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/dashdec.c |  8 ++++----
 libavformat/hls.c     | 12 ++++++------
 2 files changed, 10 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index ee40f2aa0c..55212661be 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -400,7 +400,7 @@  static void free_subtitle_list(DASHContext *c)
 }
 
 static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
-                    AVDictionary *opts, AVDictionary *opts2, int *is_http)
+                    AVDictionary **opts, AVDictionary *opts2, int *is_http)
 {
     DASHContext *c = s->priv_data;
     AVDictionary *tmp = NULL;
@@ -440,7 +440,7 @@  static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
         return AVERROR_INVALIDDATA;
 
     av_freep(pb);
-    av_dict_copy(&tmp, opts, 0);
+    av_dict_copy(&tmp, *opts, 0);
     av_dict_copy(&tmp, opts2, 0);
     ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
     if (ret >= 0) {
@@ -451,7 +451,7 @@  static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
             av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, (uint8_t**)&new_cookies);
 
         if (new_cookies) {
-            av_dict_set(&opts, "cookies", new_cookies, AV_DICT_DONT_STRDUP_VAL);
+            av_dict_set(opts, "cookies", new_cookies, AV_DICT_DONT_STRDUP_VAL);
         }
 
     }
@@ -1714,7 +1714,7 @@  static int open_input(DASHContext *c, struct representation *pls, struct fragmen
     ff_make_absolute_url(url, c->max_url_size, c->base_url, seg->url);
     av_log(pls->parent, AV_LOG_VERBOSE, "DASH request for url '%s', offset %"PRId64", playlist %d\n",
            url, seg->url_offset, pls->rep_idx);
-    ret = open_url(pls->parent, &pls->input, url, c->avio_opts, opts, NULL);
+    ret = open_url(pls->parent, &pls->input, url, &c->avio_opts, opts, NULL);
 
 cleanup:
     av_free(url);
diff --git a/libavformat/hls.c b/libavformat/hls.c
index 3ab07f1b3f..f33ff3f645 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -617,7 +617,7 @@  static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
 }
 
 static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
-                    AVDictionary *opts, AVDictionary *opts2, int *is_http_out)
+                    AVDictionary **opts, AVDictionary *opts2, int *is_http_out)
 {
     HLSContext *c = s->priv_data;
     AVDictionary *tmp = NULL;
@@ -664,7 +664,7 @@  static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
     else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5))
         return AVERROR_INVALIDDATA;
 
-    av_dict_copy(&tmp, opts, 0);
+    av_dict_copy(&tmp, *opts, 0);
     av_dict_copy(&tmp, opts2, 0);
 
     if (is_http && c->http_persistent && *pb) {
@@ -690,7 +690,7 @@  static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
             av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, (uint8_t**)&new_cookies);
 
         if (new_cookies)
-            av_dict_set(&opts, "cookies", new_cookies, AV_DICT_DONT_STRDUP_VAL);
+            av_dict_set(opts, "cookies", new_cookies, AV_DICT_DONT_STRDUP_VAL);
     }
 
     av_dict_free(&tmp);
@@ -1231,12 +1231,12 @@  static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
            seg->url, seg->url_offset, pls->index);
 
     if (seg->key_type == KEY_NONE) {
-        ret = open_url(pls->parent, in, seg->url, c->avio_opts, opts, &is_http);
+        ret = open_url(pls->parent, in, seg->url, &c->avio_opts, opts, &is_http);
     } else if (seg->key_type == KEY_AES_128) {
         char iv[33], key[33], url[MAX_URL_SIZE];
         if (strcmp(seg->key, pls->key_url)) {
             AVIOContext *pb = NULL;
-            if (open_url(pls->parent, &pb, seg->key, c->avio_opts, opts, NULL) == 0) {
+            if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, opts, NULL) == 0) {
                 ret = avio_read(pb, pls->key, sizeof(pls->key));
                 if (ret != sizeof(pls->key)) {
                     av_log(pls->parent, AV_LOG_ERROR, "Unable to read key file %s\n",
@@ -1260,7 +1260,7 @@  static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
         av_dict_set(&opts, "key", key, 0);
         av_dict_set(&opts, "iv", iv, 0);
 
-        ret = open_url(pls->parent, in, url, c->avio_opts, opts, &is_http);
+        ret = open_url(pls->parent, in, url, &c->avio_opts, opts, &is_http);
         if (ret < 0) {
             goto cleanup;
         }