From patchwork Fri Apr 24 10:54:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mattias Wadman X-Patchwork-Id: 19211 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 CA94E449C5D for ; Fri, 24 Apr 2020 13:54:53 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A7BD168C12C; Fri, 24 Apr 2020 13:54:53 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id ACACB68C07A for ; Fri, 24 Apr 2020 13:54:47 +0300 (EEST) Received: by mail-wr1-f49.google.com with SMTP id f13so10186563wrm.13 for ; Fri, 24 Apr 2020 03:54:47 -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=RVZ8ysoRTyNMuXmDxPcnRvXC2z9vg33EaFN/Vq7tSi8=; b=nVoSafNeD2u8tRVclVOuyChnt+V/H0lBXcg2DAjvftjnszqFMDq53QuG6qCrHBdVXE MGPrxxfYrTrREOuS5OqiZJWhhRaeONv4MgPR/dushQ+TTsZ3Cg8gmUJgYsueEFi1aXCj 6zsJE1FzwImJQcSG21OmU95BQxUHO/qigc3n5NtobILcnisWXjVw1+yH5Nu/xv599fSl ARDh7YhtdbKeDo0zTi9/aiSsdlK+smBmQQJTOLeXeLOwb80mbU+zOqdcerffNntvyODW 5RWFenibjqeNAPAz4P199PYuzKJHQF3UC3+l0M2ML9mVItLEd3gam46ILElQAsQhNlkl Pghw== 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=RVZ8ysoRTyNMuXmDxPcnRvXC2z9vg33EaFN/Vq7tSi8=; b=Tz0OHaptDG3IXF5Cwv34F8dbsHfi/qjuD2za+rLQ7LSMN2d4uSGi04Ww5+T6aj8JKg dDwmZlXtG2+m5H8hsK3UipPHc7NgRUioIhZ1X8xX8g4UJc5bupNih+z1plnPje51I6SF o30Dy1Wx4wcD5OXiY0TF9luHkylcaGBjvjerbrPZf/YzOdVtIsN9TbYgW3CJQk8AQtkb 2OAg+jpc5EDa37espla+KgK/U3qPTsSDnw0pl9Kt7TbQxoUKTIDEYBE5t8zT0vhuYywB pA5Bshneni7wG196FnDebP8e+LtT9Oq8BtzlDfXDFSneCetPMZ2Lgal/Ik2+Wrf+9XsQ hw4Q== X-Gm-Message-State: AGi0PuYcYbxdx33sc0G3xAF/d2x8JXmn8KW+/GYpUYTW8oDMYCbGFOXN sJ/aFaIeZ+GsNkrJIoea3dko/oNgpgQ= X-Google-Smtp-Source: APiQypLrQK9gwzcAXr+eftTqfPIeE4+ee3jOiNI+OcaHY+1yYBrIvLbIPcdOCm2Ak65dKUFiMrCGNQ== X-Received: by 2002:a5d:4246:: with SMTP id s6mr10455868wrr.421.1587725686615; Fri, 24 Apr 2020 03:54:46 -0700 (PDT) Received: from work-2.local.net (c-1d85e253.016-130-73746f46.bbcust.telenor.se. [83.226.133.29]) by smtp.gmail.com with ESMTPSA id z22sm2285050wma.20.2020.04.24.03.54.45 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Apr 2020 03:54:45 -0700 (PDT) From: Mattias Wadman To: ffmpeg-devel@ffmpeg.org Date: Fri, 24 Apr 2020 12:54:37 +0200 Message-Id: <20200424105437.69082-1-mattias.wadman@gmail.com> X-Mailer: git-send-email 2.18.0 Subject: [FFmpeg-devel] [PATCH v3] 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" lavf flacenc could previously write truncated metadata picture size if the picture data did 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. Also only enable this workaround flac files and not ogg files with flac METADATA_BLOCK_PICTURE comment. flacenc was fixed in e447a4d112bcfee10126c54eb4481fa8712957c8 before the fix a broken flac 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 | 35 +++++++++++++++++++++++++++-------- libavformat/flac_picture.h | 2 +- libavformat/flacdec.c | 2 +- libavformat/oggparsevorbis.c | 2 +- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c index 81ddf80465..61277e9dee 100644 --- a/libavformat/flac_picture.c +++ b/libavformat/flac_picture.c @@ -27,7 +27,7 @@ #include "id3v2.h" #include "internal.h" -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) { const CodecMime *mime = ff_id3v2_mime_tags; enum AVCodecID id = AV_CODEC_ID_NONE; @@ -36,7 +36,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 +115,34 @@ 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) { + // Workaround lavf flacenc bug that allowed writing truncated metadata picture block size if + // picture size did not fit in 24 bits + if (truncate_workaround && len > left && (len & 0xffffff) == left) { + av_log(s, AV_LOG_INFO, "Correcting truncated metadata picture size from %d to %d\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 detect 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");