From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 9991A45DBE for ; Mon, 12 Jun 2023 11:56:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id ADA9868C379; Mon, 12 Jun 2023 14:56:45 +0300 (EEST) Received: from out162-62-57-64.mail.qq.com (out162-62-57-64.mail.qq.com [162.62.57.64]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 8374A68C176 for ; Mon, 12 Jun 2023 14:56:38 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1686570989; bh=NM6t7zaOZTGFjdyIDOKwQUanEN0tkL4xjWz2r/j3rRw=; h=From:To:Cc:Subject:Date; b=K4E/kaXW4Uu3sxkptevG9HNNhh+A8FlpJfS5OkeA7M7JUyWmGDVXHR0gkF7MjScjO kPFO753ptCmGyUocjxU2hMWpA8SqcE8+K8veyafK9XMmPUSxGL9zXenQ1gHw+Pcj7Q 23zgFbU5PcbF9cUwIWUae6M5EY4F4LOE7U9XIB04= Received: from localhost.localdomain ([119.147.10.203]) by newxmesmtplogicsvrszb9-0.qq.com (NewEsmtp) with SMTP id E1C1BE89; Mon, 12 Jun 2023 19:56:28 +0800 X-QQ-mid: xmsmtpt1686570988tp8bahskf Message-ID: X-QQ-XMAILINFO: M8F5Ju5R+mONu70YcMsKxibnM04nrzFqIrgjhE91nLDPWwxdGZn3eo1X4YigHt +PtfVjcHJGIUYUqVyWxmLDrWmdo4tPl0VKdoN4IkCge9W14qzWjWT6CzG85e8pSIxX/LQFacMgt5 DnWaYU9L1ZYL2URcLxflRNw3LyN5sODfsYJ0a5YYqDZ68UdTuF4/D5UpzVajSiI1C6sHgimXXOFT iO3igGvQtVALtIHBr7bZl6dKH2omGDWH56ScI5Vcy2XqTjKxgn7M5uzdsn6CDFB6Mqw6cWGPccd/ N6MbVqygEn9zc3mK20yXO5BXDwfpgWO09/AMXOMnV4NH1pNh60iU8GvIc9rgfBI3kIhrv/c26emS W+70ujBzrC6Vk5dFn6GDkJzdbdlSbW6mYSKzpsimDSDbA+ZhrMRX4G6ask07pkYxE2VhgbA+bQmZ LUlGVqMrnYFkAWmpd0O4dqXqzC+E/bMZwHXkYqCrqqedSMrBBiVSSMBv3Ot1/vZmckO3PzI9zF2p fj3FTG5cC/Pmb3TJAXNghEQRujz4LhALbZX7aoSxhXUdMO2SBoYxUZA0YTqSCw5xXovBosg7M1ww gRKscH0EfuCxc+1AduNkF3k3U9tZPUFSDW1FIECbggCC5vQEO24hTeONEH6DbHGxA0Kn87Bo3zmu riK3N6DavHbWwu52EekmBIgJGx20s4m3z9r4ADgO7hs8jlFFOlSw35nrwt3DWSqbJLs6H9ga/K46 lGo93DpKECLSrMmYCORtedKIfgyJq6GhrcLXYU0r3WSC/AafFtKvn6OG0diaYmZhJEUkq1QB2VV5 xZNa03526s0MXJEHeubf7dljIbLq1wqLunRzpji1VVP2KT3DPTfQ7683OJYfTT4V335Olw6xC+9L FYOovBjw3QbNZLGIBDDXCm8loGb+YJV4AfhBHQgAxmSgtQqyPPJUKH8uW/mTZyq4kFZm9+5TnAEy CCpHYwDOLWCIfGat1kShtmuscIemRitTsz0pm42k4= X-QQ-XMAILREADINFO: Nho4V/E0OEdrqQ11svVYjpA= From: Zhao Zhili To: ffmpeg-devel@ffmpeg.org Date: Mon, 12 Jun 2023 19:56:28 +0800 X-OQ-MSGID: <20230612115628.237937-1-quinkblack@foxmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avformat/mov: fix overallocation when reading pssh/saiz X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 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: Zhao Zhili Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: From: Zhao Zhili mov_try_read_block() allocates 1MB at least, which can be more than enough. It was called when reading saiz box, which can appear periodically inside fmp4. This consumes a lot of memory. We can fix mov_try_read_block() by clamp 'block_size' with 'size'. However, the function is harmful than helpful. It avoids allocating large memory when the real data is small. Even in that case, if allocating large memory directly failed, it's fine to return ENOMEM; if allocating success and reading doesn't match the given size, it's fine to free and return AVERROR_INVALIDDATA. In other cases, it's a waste of CPU and memory. So I decided to remove the function, and replace it by call av_malloc() and avio_read() directly. mov_read_saiz() and mov_read_pssh() need more check, but they don't belong to this patch. Fixes #7641 and #9243. Signed-off-by: Zhao Zhili --- libavformat/mov.c | 63 +++++++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/libavformat/mov.c b/libavformat/mov.c index a8d004e02b..3d0969545a 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -6649,38 +6649,6 @@ finish: return ret; } -/** - * Tries to read the given number of bytes from the stream and puts it in a - * newly allocated buffer. This reads in small chunks to avoid allocating large - * memory if the file contains an invalid/malicious size value. - */ -static int mov_try_read_block(AVIOContext *pb, size_t size, uint8_t **data) -{ - const unsigned int block_size = 1024 * 1024; - uint8_t *buffer = NULL; - unsigned int alloc_size = 0, offset = 0; - while (offset < size) { - unsigned int new_size = - alloc_size >= INT_MAX - block_size ? INT_MAX : alloc_size + block_size; - uint8_t *new_buffer = av_fast_realloc(buffer, &alloc_size, new_size); - unsigned int to_read = FFMIN(size, alloc_size) - offset; - if (!new_buffer) { - av_free(buffer); - return AVERROR(ENOMEM); - } - buffer = new_buffer; - - if (avio_read(pb, buffer + offset, to_read) != to_read) { - av_free(buffer); - return AVERROR_INVALIDDATA; - } - offset += to_read; - } - - *data = buffer; - return 0; -} - static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) { MOVEncryptionIndex *encryption_index; @@ -6736,15 +6704,24 @@ static int mov_read_saiz(MOVContext *c, AVIOContext *pb, MOVAtom atom) encryption_index->auxiliary_info_default_size = avio_r8(pb); sample_count = avio_rb32(pb); - encryption_index->auxiliary_info_sample_count = sample_count; if (encryption_index->auxiliary_info_default_size == 0) { - ret = mov_try_read_block(pb, sample_count, &encryption_index->auxiliary_info_sizes); - if (ret < 0) { - av_log(c->fc, AV_LOG_ERROR, "Failed to read the auxiliary info\n"); + encryption_index->auxiliary_info_sizes = av_malloc(sample_count); + if (!encryption_index->auxiliary_info_sizes) + return AVERROR(ENOMEM); + + ret = avio_read(pb, encryption_index->auxiliary_info_sizes, sample_count); + if (ret != sample_count) { + av_freep(&encryption_index->auxiliary_info_sizes); + + if (ret >= 0) + ret = AVERROR_INVALIDDATA; + av_log(c->fc, AV_LOG_ERROR, "Failed to read the auxiliary info, %s\n", + av_err2str(ret)); return ret; } } + encryption_index->auxiliary_info_sample_count = sample_count; if (encryption_index->auxiliary_offsets_count) { return mov_parse_auxiliary_info(c, sc, pb, encryption_index); @@ -6913,9 +6890,19 @@ static int mov_read_pssh(MOVContext *c, AVIOContext *pb, MOVAtom atom) } extra_data_size = avio_rb32(pb); - ret = mov_try_read_block(pb, extra_data_size, &extra_data); - if (ret < 0) + extra_data = av_malloc(extra_data_size); + if (!extra_data) { + ret = AVERROR(ENOMEM); goto finish; + } + ret = avio_read(pb, extra_data, extra_data_size); + if (ret != extra_data_size) { + av_free(extra_data); + + if (ret >= 0) + ret = AVERROR_INVALIDDATA; + goto finish; + } av_freep(&info->data); // malloc(0) may still allocate something. info->data = extra_data; -- 2.25.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".