From patchwork Tue May 19 09:27:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mattias Wadman X-Patchwork-Id: 19768 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 3742444AA29 for ; Tue, 19 May 2020 12:27:35 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0CF546881A0; Tue, 19 May 2020 12:27:35 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 02D34688136 for ; Tue, 19 May 2020 12:27:28 +0300 (EEST) Received: by mail-wm1-f44.google.com with SMTP id f134so2312106wmf.1 for ; Tue, 19 May 2020 02:27:28 -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; bh=PaR3f7MFGBOJUfaKH0QvL97fLbMO+iUJ5tE08tDxGQU=; b=qJLRQlsUOP2ZPm8wnu8n8jpkIeV4OqjtkpW+M4qjWDKipjtALeA6VcxRx0PReeQYkx imU+EQp+ATHzNvPrkqEsl7HyL3U64urnXFW7LpKvdBFp7CXXtpDkJCByZ8ZywQRuPOqP /UbeplMtiMeTnlObck685xq7h37F0J6yRCh+wwGQkXI2wGPkqfvJpyKBwHkMPiJF3v9h 5rF7JBJnEvNcxoK5Wm2OsaQMhf3w1EzfnV+I4jlo/iJSIFZGMKQ02M+OYxR4VZdHduVr 8csNLHcDrXbltkELBHMgzN4RpXEr9dnrzx5uVxXsSGV3I8BczkYar/RN7fsAlEHAbAm6 iFYA== 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; bh=PaR3f7MFGBOJUfaKH0QvL97fLbMO+iUJ5tE08tDxGQU=; b=kzWECefWdQuXHS648YYazDVNraVkcmkq03/K97z3G7mgfCa3bKFhkiklQ7eB3kONAL o1IAnRK7S6rBY53WYWlkuGM1U+1NK1rOgGC3yTZpyi45x0IIRXti2cEa/Z4BkI0yjzoG CN/KNIGis8qtPeO2vnBQFsbUhXS+qb9Aifr6uPLuxWG9srCAmTgvH5pEXhn8EcOpycTc 7FJZu2P23hsR+pbG3Cda3X40wcgZ4Ids8SSiscaSRH37pLayHmyn0Ujj4bdpTq9/Ps6n mP3nDLp0bH2c/G66Nq0GSVPV6P96r9m6ZL7mOzv0+/I5gt0s6jtKC/R+Yp1jFmqb2tzx abAg== X-Gm-Message-State: AOAM530Ql9Mk1pk4RySbKgNiVG6VR+BPkZdurf4d/zN2HB9jEN2ZosKV hqbFCKaRfgdyjmp4hJIe5SZp0CxsZSc= X-Google-Smtp-Source: ABdhPJw1i0q1GUZmPmf7+7fBjsDNV3pYg5SsS2bXFlvWEPSdZzQGdjOTcRa6xnJiXIyc04Zaos+Uog== X-Received: by 2002:a1c:29c4:: with SMTP id p187mr4346236wmp.73.1589880447761; Tue, 19 May 2020 02:27:27 -0700 (PDT) Received: from work-2.local.net (c-388de253.016-130-73746f46.bbcust.telenor.se. [83.226.141.56]) by smtp.gmail.com with ESMTPSA id s17sm2968856wmc.48.2020.05.19.02.27.26 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 May 2020 02:27:27 -0700 (PDT) From: Mattias Wadman To: ffmpeg-devel@ffmpeg.org Date: Tue, 19 May 2020 11:27:11 +0200 Message-Id: <20200519092710.72859-1-mattias.wadman@gmail.com> X-Mailer: git-send-email 2.18.0 Subject: [FFmpeg-devel] [PATCH v5] libavformat/flacdec: Workaround for truncated metadata picture size 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: Mattias Wadman MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Some flac muxers write truncated metadata picture size if the picture data do not fit in 24 bits. Detect this by truncting the size found inside the picture block and if it matches the block size use it and read rest of picture data. This workaround is only for flac files and not ogg files with flac METADATA_BLOCK_PICTURE comments and it can be disabled with strict level above normal. Currently there is a 500MB limit on truncate size to protect from large memory allocations. The truncation bug in lavf flacenc was fixed in e447a4d112bcfee10126c54eb4481fa8712957c8 but based on existing broken files other unknown flac muxers seems to truncate also. Before the fix a broken flac file for reproduction could be generated with: ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 -map 1:0 -c:v:0 bmp -disposition:1 attached_pic -t 1 test.flac Fixes ticket 6333 --- libavformat/flac_picture.c | 47 ++++++++++++++++++++++++++++++------ libavformat/flac_picture.h | 2 +- libavformat/flacdec.c | 2 +- libavformat/oggparsevorbis.c | 2 +- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c index 81ddf80465..53e24b28b7 100644 --- a/libavformat/flac_picture.c +++ b/libavformat/flac_picture.c @@ -27,7 +27,9 @@ #include "id3v2.h" #include "internal.h" -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) +#define MAX_TRUNC_PICTURE_SIZE (500 * 1024 * 1024) + +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int truncate_workaround) { const CodecMime *mime = ff_id3v2_mime_tags; enum AVCodecID id = AV_CODEC_ID_NONE; @@ -36,7 +38,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) GetByteContext g; AVStream *st; int width, height, ret = 0; - unsigned int len, type; + unsigned int type; + uint32_t len, left, trunclen = 0; if (buf_size < 34) { av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); @@ -114,16 +117,44 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) /* picture data */ len = bytestream2_get_be32u(&g); - if (len <= 0 || len > bytestream2_get_bytes_left(&g)) { - av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); - if (s->error_recognition & AV_EF_EXPLODE) - ret = AVERROR_INVALIDDATA; - goto fail; + + left = bytestream2_get_bytes_left(&g); + if (len <= 0 || len > left) { + if (len > MAX_TRUNC_PICTURE_SIZE || len >= INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) { + av_log(s, AV_LOG_ERROR, "Attached picture metadata block too big %u\n", len); + if (s->error_recognition & AV_EF_EXPLODE) + ret = AVERROR_INVALIDDATA; + goto fail; + } + + // Workaround bug for flac muxers that writs truncated metadata picture block size if + // the picture size do not fit in 24 bits. lavf flacenc used to have the issue and based + // on existing broken files other unknown flac muxers seems to truncate also. + if (truncate_workaround && + s->strict_std_compliance <= FF_COMPLIANCE_NORMAL && + len > left && (len & 0xffffff) == left) { + av_log(s, AV_LOG_INFO, "Correcting truncated metadata picture size from %u to %u\n", left, len); + trunclen = len - left; + } else { + av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); + if (s->error_recognition & AV_EF_EXPLODE) + ret = AVERROR_INVALIDDATA; + goto fail; + } } if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) { RETURN_ERROR(AVERROR(ENOMEM)); } - bytestream2_get_bufferu(&g, data->data, len); + + if (trunclen == 0) { + bytestream2_get_bufferu(&g, data->data, len); + } else { + // If truncation was detected copy all data from block and read missing bytes + // not included in the block size + bytestream2_get_bufferu(&g, data->data, left); + if (avio_read(s->pb, data->data + len - trunclen, trunclen) < trunclen) + RETURN_ERROR(AVERROR_INVALIDDATA); + } memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE); if (AV_RB64(data->data) == PNGSIG) diff --git a/libavformat/flac_picture.h b/libavformat/flac_picture.h index 4374b6f4f6..61fd0c8806 100644 --- a/libavformat/flac_picture.h +++ b/libavformat/flac_picture.h @@ -26,6 +26,6 @@ #define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0) -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size); +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, int truncate_workaround); #endif /* AVFORMAT_FLAC_PICTURE_H */ diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c index cb516fb1f3..79c05f14bf 100644 --- a/libavformat/flacdec.c +++ b/libavformat/flacdec.c @@ -146,7 +146,7 @@ static int flac_read_header(AVFormatContext *s) } av_freep(&buffer); } else if (metadata_type == FLAC_METADATA_TYPE_PICTURE) { - ret = ff_flac_parse_picture(s, buffer, metadata_size); + ret = ff_flac_parse_picture(s, buffer, metadata_size, 1); av_freep(&buffer); if (ret < 0) { av_log(s, AV_LOG_ERROR, "Error parsing attached picture.\n"); diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c index 27d2c686b6..6f15204ada 100644 --- a/libavformat/oggparsevorbis.c +++ b/libavformat/oggparsevorbis.c @@ -165,7 +165,7 @@ int ff_vorbis_comment(AVFormatContext *as, AVDictionary **m, av_freep(&tt); av_freep(&ct); if (ret > 0) - ret = ff_flac_parse_picture(as, pict, ret); + ret = ff_flac_parse_picture(as, pict, ret, 0); av_freep(&pict); if (ret < 0) { av_log(as, AV_LOG_WARNING, "Failed to parse cover art block.\n");