From patchwork Fri Nov 29 19:44:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 16489 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 D33EC449BF4 for ; Fri, 29 Nov 2019 21:44:37 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B880568B43F; Fri, 29 Nov 2019 21:44:37 +0200 (EET) 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 3C5C368B1E3 for ; Fri, 29 Nov 2019 21:44:31 +0200 (EET) Received: by mail-wr1-f68.google.com with SMTP id g17so4456102wro.2 for ; Fri, 29 Nov 2019 11:44:31 -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=jfGttAK76SbuBW4s7bKrbXZWULvHSoy/O0BTXprQiDw=; b=Ek6u/6OYtrmccZW9SnXb+oYQp3rAJccPwf5gZZLzpyKdsY4HUFb5UsvIjzOBx4L3RU c49e8QONOqWfDsKKuaOjaeAFd4EEc4vnhydLtFozk0ig0jN70NXRPq7O5klN1rUKPpMZ emjAkimwCkukuhILTOQObYj8gNoBG2gNnbisn+VDL6E8C+QnKfmYwxAnFyO4Oe5FeIk9 jYAzZGXqQiTDAzaoU+EfAWg4FiuGGwCpSNyALuf072VbUixJN/5FBdPw+BIssvw3Mc6d 18o3JoxJqyrZbkwsMSEhFHPCReGcvNoDZ3uUXFBCnM5ZQMhnNjGF2yPPLF5ErmODs+oR d9cQ== 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=jfGttAK76SbuBW4s7bKrbXZWULvHSoy/O0BTXprQiDw=; b=A8ER323w2uYhb/XFM4QbeoJmvfIvoYe9QSqeB+1N2q2PcbWYleeh6Hr9OmUFSBJPUT s+H9Pkg6vczuvgXdJTK/mgO1o1xaRiaVzioTmKF3Kgec0wfI757UM9r1/j8YSe2IJPvW Aq9pIAEBvLLccSdAhFRxnqCvFTYhEqV8OW8QxAUZZ+/TNQnCB/vgx33ajA/k7x0h6YP1 g2KZha5tT5LcdTL9LLsylFrFZ0LZwcjc5Fi9bM0LRFHz9mWZUFPVgPp3ShCgSrjndfmV EUhEBYl8i7xn5SN6X/KFsY2pIKUJxWXqZ8JZCjxsSWVGVHCwgg5jesxc0BJBIYH0Qy5V QqUQ== X-Gm-Message-State: APjAAAVsiLgg7CYMhc/QYUPozPEuokP77cQ3VIdexZilvpIXOPeQnAk7 DHTHRlzHI1RA4ffJpRS5L+umnFas X-Google-Smtp-Source: APXvYqyyWqYs4Gsgt0VAvrzwESpOCgoGb+6HNTDSmhpVXdObn8VmIVw4iczlh9xRhK1EVTll1k2GJw== X-Received: by 2002:adf:dc06:: with SMTP id t6mr57856154wri.378.1575056670561; Fri, 29 Nov 2019 11:44:30 -0800 (PST) Received: from sblaptop.fritz.box (ipbcc08e23.dynamic.kabel-deutschland.de. [188.192.142.35]) by smtp.gmail.com with ESMTPSA id x7sm28059531wrq.41.2019.11.29.11.44.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Nov 2019 11:44:30 -0800 (PST) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 29 Nov 2019 20:44:10 +0100 Message-Id: <20191129194411.20168-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191129194411.20168-1-andreas.rheinhardt@gmail.com> References: <20191129194411.20168-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/3] avformat/flac_picture: Switch to bytestream2 API 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" ff_flac_parse_picture() parses a buffer containing a flac metadata picture block by wrapping it in an AVIOContext and using the AVIOContext API. Consequently, when not enough data could be read AVERROR(EIO) was returned although reading didn't really fail: A block that contains a subfield whose size field indicates that it is so big as to extend beyond the buffer is just invalid. This commit changes this by using the bytestream2 API instead; furthermore, the checks for whether there is enough data left are performed before allocating a buffer for said data. Finally, if the length of the picture description is bigger than INT_MAX, it will now raise an error. Signed-off-by: Andreas Rheinhardt --- libavformat/flac_picture.c | 64 +++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c index 6463a370c8..ccb0ee613e 100644 --- a/libavformat/flac_picture.c +++ b/libavformat/flac_picture.c @@ -20,12 +20,12 @@ */ #include "libavutil/intreadwrite.h" +#include "libavcodec/bytestream.h" #include "libavcodec/png.h" #include "avformat.h" #include "flac_picture.h" #include "id3v2.h" #include "internal.h" -#include "avio_internal.h" int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) { @@ -33,16 +33,22 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) enum AVCodecID id = AV_CODEC_ID_NONE; AVBufferRef *data = NULL; uint8_t mimetype[64], *desc = NULL; - AVIOContext pb0, *pb = &pb0; + GetByteContext g; AVStream *st; int width, height, ret = 0; - int len; - unsigned int type; + unsigned int len, type; - ffio_init_context(pb, buf, buf_size, 0, NULL, NULL, NULL, NULL); + if (buf_size < 34) { + av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); + if (s->error_recognition & AV_EF_EXPLODE) + return AVERROR_INVALIDDATA; + return 0; + } + + bytestream2_init(&g, buf, buf_size); /* read the picture type */ - type = avio_rb32(pb); + type = bytestream2_get_be32u(&g); if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types)) { av_log(s, AV_LOG_ERROR, "Invalid picture type: %d.\n", type); if (s->error_recognition & AV_EF_EXPLODE) { @@ -52,15 +58,21 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) } /* picture mimetype */ - len = avio_rb32(pb); - if (len <= 0 || len >= sizeof(mimetype) || - avio_read(pb, mimetype, len) != len) { + len = bytestream2_get_be32u(&g); + if (len <= 0 || len >= sizeof(mimetype)) { av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached " "picture.\n"); if (s->error_recognition & AV_EF_EXPLODE) ret = AVERROR_INVALIDDATA; goto fail; } + if (len + 24 > 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) + return AVERROR_INVALIDDATA; + return 0; + } + bytestream2_get_bufferu(&g, mimetype, len); mimetype[len] = 0; while (mime->id != AV_CODEC_ID_NONE) { @@ -79,30 +91,31 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) } /* picture description */ - len = avio_rb32(pb); + len = bytestream2_get_be32u(&g); + if (len > bytestream2_get_bytes_left(&g) - 20) { + av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n"); + if (s->error_recognition & AV_EF_EXPLODE) + return AVERROR_INVALIDDATA; + return 0; + } if (len > 0) { if (!(desc = av_malloc(len + 1))) { RETURN_ERROR(AVERROR(ENOMEM)); } - if (avio_read(pb, desc, len) != len) { - av_log(s, AV_LOG_ERROR, "Error reading attached picture description.\n"); - if (s->error_recognition & AV_EF_EXPLODE) - ret = AVERROR(EIO); - goto fail; - } + bytestream2_get_bufferu(&g, desc, len); desc[len] = 0; } /* picture metadata */ - width = avio_rb32(pb); - height = avio_rb32(pb); - avio_skip(pb, 8); + width = bytestream2_get_be32u(&g); + height = bytestream2_get_be32u(&g); + bytestream2_skipu(&g, 8); /* picture data */ - len = avio_rb32(pb); - if (len <= 0) { - av_log(s, AV_LOG_ERROR, "Invalid attached picture size: %d.\n", len); + 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; @@ -110,13 +123,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size) if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) { RETURN_ERROR(AVERROR(ENOMEM)); } + bytestream2_get_bufferu(&g, data->data, len); memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE); - if (avio_read(pb, data->data, len) != len) { - av_log(s, AV_LOG_ERROR, "Error reading attached picture data.\n"); - if (s->error_recognition & AV_EF_EXPLODE) - ret = AVERROR(EIO); - goto fail; - } if (AV_RB64(data->data) == PNGSIG) id = AV_CODEC_ID_PNG;