diff mbox series

[FFmpeg-devel,13/21] avformat/dashdec: Fix leak of string on error when parsing representation

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

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:36 p.m. UTC 
The DASH demuxer currently extracts several strings at once from an xml
document before processing them one by one; these strings are allocated,
stored in local variables and need to be freed by the demuxer itself.
So if an error happens when processing one of them, all strings need to
be freed before returning. This has simply not been done, leading to
leaks.

A simple fix would be to add the necessary code for freeing; yet there is
a better solution: Avoid having several strings at the same time by
extracting a string, processing it and immediately freeing it. That way
one only has to free at most one string on error.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/dashdec.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 90d0e89925..ca2c2b5fd2 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -897,46 +897,45 @@  static int parse_manifest_representation(AVFormatContext *s, const char *url,
             fragment_templates_tab[3] = period_segmenttemplate_node;
             fragment_templates_tab[4] = period_segmentlist_node;
 
-            presentation_timeoffset_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "presentationTimeOffset");
-            duration_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "duration");
-            startnumber_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "startNumber");
-            timescale_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "timescale");
             initialization_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "initialization");
-            media_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "media");
-
             if (initialization_val) {
                 rep->init_section = av_mallocz(sizeof(struct fragment));
-                if (!rep->init_section)
+                if (!rep->init_section) {
+                    xmlFree(initialization_val);
                     goto enomem;
+                }
                 c->max_url_size = aligned(c->max_url_size  + strlen(initialization_val));
                 rep->init_section->url = get_content_url(baseurl_nodes, 4,  c->max_url_size, rep_id_val, rep_bandwidth_val, initialization_val);
+                xmlFree(initialization_val);
                 if (!rep->init_section->url)
                     goto enomem;
                 rep->init_section->size = -1;
-                xmlFree(initialization_val);
             }
-
+            media_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "media");
             if (media_val) {
                 c->max_url_size = aligned(c->max_url_size  + strlen(media_val));
                 rep->url_template = get_content_url(baseurl_nodes, 4, c->max_url_size, rep_id_val, rep_bandwidth_val, media_val);
                 xmlFree(media_val);
             }
-
+            presentation_timeoffset_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "presentationTimeOffset");
             if (presentation_timeoffset_val) {
                 rep->presentation_timeoffset = (int64_t) strtoll(presentation_timeoffset_val, NULL, 10);
                 av_log(s, AV_LOG_TRACE, "rep->presentation_timeoffset = [%"PRId64"]\n", rep->presentation_timeoffset);
                 xmlFree(presentation_timeoffset_val);
             }
+            duration_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "duration");
             if (duration_val) {
                 rep->fragment_duration = (int64_t) strtoll(duration_val, NULL, 10);
                 av_log(s, AV_LOG_TRACE, "rep->fragment_duration = [%"PRId64"]\n", rep->fragment_duration);
                 xmlFree(duration_val);
             }
+            timescale_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "timescale");
             if (timescale_val) {
                 rep->fragment_timescale = (int64_t) strtoll(timescale_val, NULL, 10);
                 av_log(s, AV_LOG_TRACE, "rep->fragment_timescale = [%"PRId64"]\n", rep->fragment_timescale);
                 xmlFree(timescale_val);
             }
+            startnumber_val = get_val_from_nodes_tab(fragment_templates_tab, 4, "startNumber");
             if (startnumber_val) {
                 rep->start_number = rep->first_seq_no = (int64_t) strtoll(startnumber_val, NULL, 10);
                 av_log(s, AV_LOG_TRACE, "rep->first_seq_no = [%"PRId64"]\n", rep->first_seq_no);