From patchwork Mon Jun 12 11:56:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhao Zhili X-Patchwork-Id: 42040 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a20:c526:b0:117:ac03:c9de with SMTP id gm38csp2908565pzb; Mon, 12 Jun 2023 04:56:51 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4HaxtMzS7SKwxhU8xFH/E7zq+pJGa/g0NPyjyZ+nUaGtTxEJZNsLUUDWqDJe0xN567lPpG X-Received: by 2002:a19:5001:0:b0:4f2:769a:120e with SMTP id e1-20020a195001000000b004f2769a120emr3666497lfb.2.1686571010900; Mon, 12 Jun 2023 04:56:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686571010; cv=none; d=google.com; s=arc-20160816; b=G9KsPkkP8NFg9Ed6KjhaJmH2RcI++K5RxdOw11B83TA6xjd4FxMPhbcuZHNtYBwj6d OGdYVChaGu0CxU4EdzvyIZAC88vLAv8ur0dmjC7h916ArBEFVRkEAsPi6IY0+Kmq2VC6 N1nI4r86K6M6knIrRvo0XOVqO0PzZuWYBKeNqlr08BFOwZyjpL3IfkF5vwpkODuoqxHp XSTcAlHgXNjpSxzbm/2gBdmKpDG/xf1JSGUL2mDkl9neUCO/Q5oTH2UXcBSaQDDvQ6SB P6tf7nb8Vi4+Be8V7KbNFaeVgTXwlunFAroM5kzz+i4gfhrP4Zc3RTlZ0GU2qdo2GASA QXrQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:date:to:from:message-id :dkim-signature:delivered-to; bh=7e6k+jAU4gspgEzGNMvTRXslsD9IS2XZg8Jz3OiI8kQ=; b=Mj5jsCApixsOMWePDhhx7RFufZNrtv9quJUVVQUrME/8vuZZJxbo7bECqasqhBLqFc ITNlx8CjZ1N1odJYlPmD4pZ3JRWXAJFZkqisyWsXK8zBY4ba2xgkDM47wQ4co9iY9+ex gMVEd+yeftvWLKRM7YBtwfiq/ltJaA3QAUn3qReU1BXw9Bqaly6T/4hhgkq0KhPjYyZ+ px2xRLIcE7hg/p1fNB+8v7Q0NYLmSEt/rmI06KNpRASSr1Lf6Yj1xdQkzbAvZ8gxlvYY fUa08iuVkg3NLU0c2PtRTvvTnSokpS7wotL2ROAed85BeZPKJ0DSE1zWuN0GrpDrhdnU keng== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@foxmail.com header.s=s201512 header.b="K4E/kaXW"; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=foxmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id t8-20020a056402020800b005163054e337si5761147edv.221.2023.06.12.04.56.50; Mon, 12 Jun 2023 04:56:50 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@foxmail.com header.s=s201512 header.b="K4E/kaXW"; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=foxmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E277968C3B9; Mon, 12 Jun 2023 14:56:45 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: oA02ykxDOLFM 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;