From patchwork Sat Sep 19 16:36:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22493 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 09D3B44A83E for ; Sat, 19 Sep 2020 19:37:33 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E9A2A68B7E1; Sat, 19 Sep 2020 19:37:32 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8694A68B76F for ; Sat, 19 Sep 2020 19:37:26 +0300 (EEST) Received: by mail-wr1-f68.google.com with SMTP id w5so8580181wrp.8 for ; Sat, 19 Sep 2020 09:37:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=YdFQ5NSbZW3048RElGO/hCLBanJYxNnAxL2odliYE48=; b=Jr9F1db5Ly55v+DlJ2uk6VX9jouBvDxDfy+bupg0u7ksfH2yosHYvLUz8MOuTbEX9r yJJ+SNPHugTIxirSk7LmDHtlJNtyronQIF9Yj21EKiJl0U4q1J/UHoLhRfDB4batYTsk JNZ6Js0R+5kayWcSnzYosZETStkpyluz3V1EgYJ2F6/Yr5TjzAZLgwEn8MbbwqjgPZW0 d7sKXV9FG9WT+zh/sHNEMf4+acs+CfUzYROEVzcIx4UHGvWMoB1aAnrWjQReho/eJDp8 rf4Jd81Co6aYqY9xKbEMEso3lTulGxRcpHysWo2Q8+Qj2t0aa2Gx0pu+1Ka0nxGcEm2E sb9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YdFQ5NSbZW3048RElGO/hCLBanJYxNnAxL2odliYE48=; b=aWsJraBHHl2lGlTDUgVgbnu1DGuaZoEBgvU5uz/5VHOmXhThJkogYX5NAXiFQb8pvH OGr6ixtPDQdfAUCKkiMsz+gEzIf11GCRZ2IfAACNbAVLPIybw06l+6S6yi2DSqtdu+J+ 8+Lf5iinkRoRzbmXSD3u3ES3rADQYNZNLsYZW2ggRNBSw4nJTaecf5fv4mBKctRTCVv+ Jykb78Ynn7lFA6YFqzNd36kR7yESVgnvDcs7M13p7wnkJg2bXDaJKFvIN1+HjaM/n24F +WyVQWl9USmZOGNjVRvSqLE7REWZR7Yt+SRvuLOE3rawOoC51IBJqEN2OmwTNUXlxI2u cGww== X-Gm-Message-State: AOAM5308Msuno8T3APSeZFQkG8iNuQfrWOwl6cSeQAX66f4UiO8EtL87 zX0bFq6nPKvsKb/vuHs6hN/SS9NznM0= X-Google-Smtp-Source: ABdhPJzHaU+z7d9Px50Po50lsnklk323X21aDuKSgC3th6EAt5gViXIwNQPqVqMbzlwtJyhRgmsVjQ== X-Received: by 2002:a5d:4e0a:: with SMTP id p10mr20487771wrt.170.1600533445567; Sat, 19 Sep 2020 09:37:25 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1fb0f.dynamic.kabel-deutschland.de. [188.193.251.15]) by smtp.gmail.com with ESMTPSA id d5sm12451137wrb.28.2020.09.19.09.37.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 19 Sep 2020 09:37:24 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 19 Sep 2020 18:36:02 +0200 Message-Id: <20200919163610.1099233-13-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200919163610.1099233-1-andreas.rheinhardt@gmail.com> References: <20200919163610.1099233-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 13/21] avformat/dashdec: Fix leak of string on error when parsing representation X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 --- libavformat/dashdec.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) 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);