From patchwork Fri Dec 6 23:16:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 16653 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 6ACAE44AD30 for ; Sat, 7 Dec 2019 01:18:15 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 584A768B6DC; Sat, 7 Dec 2019 01:18:15 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6ED4D68B6D3 for ; Sat, 7 Dec 2019 01:18:07 +0200 (EET) Received: by mail-wm1-f65.google.com with SMTP id t14so8915927wmi.5 for ; Fri, 06 Dec 2019 15:18:07 -0800 (PST) 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=qFKuZlnWMrffGcPL9H8u/UQM5bx1nvOR19a1Tsn1wFQ=; b=rfra0CXkAjBP9EyQeSvRxdzGpwU/P/2mv+yjmnVaGv3BeKWhuYspWKbEo4nwhlc4jD h85RHjUyr45TPyORA2b+TbMlj39k1o9O8TUhXvVzPsjARao7MS/KYp3C0hnJrkyoBbp6 4l6GomGJ+OnQ6LWofKCZVRAWLIMaxZeFn+oWXGqJK1eNCKYxTXL/IsKDfNyblLurGH2V ybN4rJCs+aqR7q3XXJUcc3+yMgjrPzYx3BDqCBjZrfhglLxDv0cc0oSYtqX0SK0dHur+ VhYceZ21/jdS6pbLNWWsbQl3tWCiS8rahOoI2e1X1W2XUeHgQ3wIWbAYdl/Qks4rs1Nt Jf+A== 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=qFKuZlnWMrffGcPL9H8u/UQM5bx1nvOR19a1Tsn1wFQ=; b=n5lsqYSzgdrZSQ7gWLjTCixe9xRV7E4zibProXt4HsiuuTMqb2hbt+1v4Y/M7Sj0dY /OKvpmtdMs1B1FGbpMDbeeefsUoIGuXxrlzxELL/Oayn2HoHzjhm4K4aL941aiUphO8B hy1Xbd4RTl/+Uzz8MMcFCZ8KsvOzgvvRalSgeeZ1oa+H7BLIZTj9rvYMau/xLKOiBNEF g4DIb3Tdp50Yir1r+iHJ6jP6dPKkla+EZkNXLF28V7K+MBW7Vnzw4oFJPyCbr5a9ODCF EBCh5JjEUANH64dTdFwfF9fq8Urh/1bg0cIE2K7VcE/RvScDcJEZwt8UGDAb+lOZpxph Em+A== X-Gm-Message-State: APjAAAVkHs3YgLxeYE3JgeY/sDLt2O1r/gkhGldflLN5GvtRX8Ta/755 2y37VAX3N6WXVHmeezteWS1vPXo3 X-Google-Smtp-Source: APXvYqwqkjcJIzkaZyTv98GzdEyeYQpbA8q8krBzgJdkSr72qphddKsoq7Ggt+VDqCftRLDIFUm05Q== X-Received: by 2002:a7b:c74c:: with SMTP id w12mr12632076wmk.1.1575674286733; Fri, 06 Dec 2019 15:18:06 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08e23.dynamic.kabel-deutschland.de. [188.192.142.35]) by smtp.gmail.com with ESMTPSA id j12sm18758461wrw.54.2019.12.06.15.18.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Dec 2019 15:18:06 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Sat, 7 Dec 2019 00:16:19 +0100 Message-Id: <20191206231622.13672-3-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191206231622.13672-1-andreas.rheinhardt@gmail.com> References: <20191206231622.13672-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/6] avformat/matroskadec: Fix use-after-free when demuxing ProRes 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" ProRes in Matroska is supposed to not contain the first atom header (containing a size field and the tag "icpf") and therefore the Matroska demuxer has to recreate it; this involves an allocation and copy, of course. Whether the old buffer (containing the data without the atom header) needs to be freed or not depends upon whether it is what was directly read (in which case it is owned by an AVBuffer) or whether it has been allocated when reversing the track's content compression (e.g. zlib compression) that Matroska supports. So there are three pointers involved: The one pointing to the directly read data (owned by the AVBuffer), the one pointing to the currently valid data (which coincides with the former if no content compression needed to be reverted) and the one pointing to the new data with the first atom header. The check for whether to free the second of these is simply whether the first two are different. This works mostly, but there is a complication: Some muxers don't strip the first atom header away and in this case, it is also not reinserted and no new buffer is allocated; instead, the second and the third pointers agree. In this case, one must never free the second buffer. Yet it is currently done if the track is e.g. zlib compressed. This commit fixes this. This is a regression since b8e75a2a. Signed-off-by: Andreas Rheinhardt --- A sample can be easily created by compressing ProRes_FromHaali.mkv (see https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3434/) with zlib with mkvmerge. libavformat/matroskadec.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index 080a839b6a..d1a0a07782 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -3240,11 +3240,8 @@ fail: static int matroska_parse_prores(MatroskaTrack *track, uint8_t *src, uint8_t **pdst, int *size) { - uint8_t *dst = src; - int dstlen = *size; - - if (AV_RB32(&src[4]) != MKBETAG('i', 'c', 'p', 'f')) { - dstlen += 8; + uint8_t *dst; + int dstlen = *size + 8; dst = av_malloc(dstlen + AV_INPUT_BUFFER_PADDING_SIZE); if (!dst) @@ -3254,7 +3251,6 @@ static int matroska_parse_prores(MatroskaTrack *track, uint8_t *src, AV_WB32(dst + 4, MKBETAG('i', 'c', 'p', 'f')); memcpy(dst + 8, src, dstlen - 8); memset(dst + dstlen, 0, AV_INPUT_BUFFER_PADDING_SIZE); - } *pdst = dst; *size = dstlen; @@ -3409,7 +3405,8 @@ static int matroska_parse_frame(MatroskaDemuxContext *matroska, pkt_data = wv_data; } - if (st->codecpar->codec_id == AV_CODEC_ID_PRORES) { + if (st->codecpar->codec_id == AV_CODEC_ID_PRORES && + AV_RB32(pkt_data + 4) != MKBETAG('i', 'c', 'p', 'f')) { uint8_t *pr_data; res = matroska_parse_prores(track, pkt_data, &pr_data, &pkt_size); if (res < 0) {