From patchwork Thu Nov 12 17:33:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 23597 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 DC7CB44979A for ; Thu, 12 Nov 2020 19:33:45 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id AA18568C188; Thu, 12 Nov 2020 19:33:45 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f67.google.com (mail-wm1-f67.google.com [209.85.128.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A4A0668C142 for ; Thu, 12 Nov 2020 19:33:39 +0200 (EET) Received: by mail-wm1-f67.google.com with SMTP id w24so6278109wmi.0 for ; Thu, 12 Nov 2020 09:33:39 -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:mime-version :content-transfer-encoding; bh=dEWEkSkVrC28IdDnbRIhwoFDiLgHomQ7fqJeGdY4Nzk=; b=cszyq1AuPsXBKm0Ofrg8jirpqbdvKQXqPzxl61G2EVNQZoDIf+0EgwwS51uZcUungc 515gw1IwIGhN82jCpTdl+wT/GfuQ8ag+flL5IM1ym5K1QXpkPJ34xtaDvB4KQSKXtMP4 mSwdPBxHpuhfOwH8KujsP/F03GB2EJ6yKIKPL6+Uh7S5YGZ6ZguwZP9Vklg56QWVEnJy EqkPpmGDqYYiQ4QW+3fH3sUWVDWfPTD20hY/fbL7h/sOJOjD9tjszJqmMbMcIvMRX6sB Vb6nfA3M90dQzEMjxsudgT377YXKIvhp5YJvK7jDIBWTWhUAVSkSaUIkldIcDPJbNhQZ szSg== 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:mime-version :content-transfer-encoding; bh=dEWEkSkVrC28IdDnbRIhwoFDiLgHomQ7fqJeGdY4Nzk=; b=VEAwXAlWyKeKzCEocRx46PJJSDvwekESYZCNTvef6x9tpAWMCOgaM517vR/Hg9U26Z m5fSNqAYaywhzVnIeM4gKV/hlDJxHEnZq41skLwQaOGy0sAv7Ym3EY5AGXJ6ouZqkc9Z HjP+PzrgJlZpqoazHCv0Q4NF4lPrxqfn2vbHxu+cAmzN578Jjtcj8CnpmnggmHFDytep Ys5D230kJ6BA+4Z7Sp32MFdcP5KJc93cLk7wLdduglooJmWTb6SPItEMXt0sF7L3CJ5s HJ00Hqa+Y1vmE/9YjGf3V8wQtkKtZKOXer6vFFNtfqLn/Un/NgDkxZBN3VuRivB+tqqL cppg== X-Gm-Message-State: AOAM532NO5i0lsRmgg24GA+zC2UrOtHg4wyCb0GkKjmWcCP0iUSneTBB fw41zrB0fTh1Its2ILYTOQbiBPL/+/j15Q== X-Google-Smtp-Source: ABdhPJyMlmU/XSWFHLaJpvCbwr9cIszU4wSidj+0W+tqPkVZ480jwk7JhXKq+LNcdFTW1ybUFwCgGg== X-Received: by 2002:a1c:80cb:: with SMTP id b194mr716153wmd.73.1605202418827; Thu, 12 Nov 2020 09:33:38 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id n15sm7814080wrq.48.2020.11.12.09.33.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Nov 2020 09:33:38 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 12 Nov 2020 18:33:25 +0100 Message-Id: <20201112173328.742491-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/4] avformat/asfdec_o: Don't segfault with lots of attached pics 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 ASF file format has a limit of 127 streams and the "asf_o" demuxer (the ASF demuxer from Libav) has an array of pointers for a structure called ASFStream that is allocated on demand for every stream. Attached pictures are not streams in the sense of the ASF specification, yet the demuxer created an ASFStream for them; and in one codepath it also forgot to check whether the array of ASFStreams is already full. The result is a write beyond the end of the array and a segfault lateron. Fixing this is easy: Don't create ASFStreams for attached picture streams. (Other results of the current state of affairs are unnecessary allocations (of ASFStreams structures), the misparsing of valid files (there might not be enough ASFStreams left for the valid streams if attached pictures take up too many); furthermore, the ASFStreams created for attached pictures all have the stream number 0, an invalid stream number (the valid range is 1-127). This means that invalid data (packets for a stream with stream number 0) won't get rejected lateron.) Signed-off-by: Andreas Rheinhardt --- libavformat/asfdec_o.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c index b142f83541..0a7e47d8cc 100644 --- a/libavformat/asfdec_o.c +++ b/libavformat/asfdec_o.c @@ -357,7 +357,6 @@ static int asf_set_metadata(AVFormatContext *s, const uint8_t *name, * but in reality this is only loosely similar */ static int asf_read_picture(AVFormatContext *s, int len) { - ASFContext *asf = s->priv_data; AVPacket pkt = { 0 }; const CodecMime *mime = ff_id3v2_mime_tags; enum AVCodecID id = AV_CODEC_ID_NONE; @@ -365,7 +364,6 @@ static int asf_read_picture(AVFormatContext *s, int len) uint8_t *desc = NULL; AVStream *st = NULL; int ret, type, picsize, desc_len; - ASFStream *asf_st; /* type + picsize + mime + desc */ if (len < 1 + 4 + 2 + 2) { @@ -422,22 +420,14 @@ static int asf_read_picture(AVFormatContext *s, int len) ret = AVERROR(ENOMEM); goto fail; } - asf->asf_st[asf->nb_streams] = av_mallocz(sizeof(*asf_st)); - asf_st = asf->asf_st[asf->nb_streams]; - if (!asf_st) { - ret = AVERROR(ENOMEM); - goto fail; - } st->disposition |= AV_DISPOSITION_ATTACHED_PIC; - st->codecpar->codec_type = asf_st->type = AVMEDIA_TYPE_VIDEO; + st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; st->codecpar->codec_id = id; st->attached_pic = pkt; - st->attached_pic.stream_index = asf_st->index = st->index; + st->attached_pic.stream_index = st->index; st->attached_pic.flags |= AV_PKT_FLAG_KEY; - asf->nb_streams++; - if (*desc) { if (av_dict_set(&st->metadata, "title", desc, AV_DICT_DONT_STRDUP_VAL) < 0) av_log(s, AV_LOG_WARNING, "av_dict_set failed.\n");