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 54CB445D14 for ; Mon, 2 Oct 2023 23:41:32 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3C62D68CADB; Tue, 3 Oct 2023 02:41:29 +0300 (EEST) Received: from mail-qk1-f177.google.com (mail-qk1-f177.google.com [209.85.222.177]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3C4EF68C9FD for ; Tue, 3 Oct 2023 02:41:22 +0300 (EEST) Received: by mail-qk1-f177.google.com with SMTP id af79cd13be357-77406094bc3so4181385a.1 for ; Mon, 02 Oct 2023 16:41:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696290080; x=1696894880; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=WQ8UqKPOK7mFYD5t7vM34vOLx/kr3x4IQVamCzOFxKw=; b=PyDV/NvvrVopaYg3KroswQMQJFiYhn3Bu0WNMeKcnXeVr4/vntJ6rrwCnQASWAWkcx ZPxKJiF5E1JEdjZhRy5ONtymXLRi6lPN050GT2ntYYfmIBrzDlh6RLl7BJtaL43o3D4f KvBkPB/brVfxaqIeYlG98WWRKLZr4ueRCNqdF1PXiXG3gkkklzIq443pStYYdzp5ln2S 83StY+g//7qhGJtP9IXRMX9dku69quYNbzSswYh0nWqPuj7Vp+EApEyhbMP40B4IQwhJ 5vrIJJSB1PC4uSHcnNNpA0BwHGLSECBF1/DvmxmCK9ExgOa/d5KpMN7nrVO/rWVAB+5F S7ng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696290080; x=1696894880; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=WQ8UqKPOK7mFYD5t7vM34vOLx/kr3x4IQVamCzOFxKw=; b=ci+PGOmJLWA5MSbbYUSIvF8nE8Q5Iehgf3ggJXAIUySqxrxzfnF4bqk9Zqq0PRZqKg 2Xj1LbZWngpG93HGfH7wxvewfanqFS25Q04KWNpUCHLaGik9H+DkDOnQAt3rZIBMh6ZM A8VkMc9ItVePhLf4yL/ifORFtyMvVbhCctpmcKPYHA8aJiD5MCEl1QTAJYCp0mMQ58do EdkokpLCQ3Bvg3ewWrfMfxaPfHeNGj6RhZKqMJl7orClOqoNo+PrrQEz184/0r+tYE61 hOzHBYnYJZZDFn4LB9uINvZ5C6/dc4Su7/ExsQFN+tmTRxlxsQisKcjrSJSVHzaqH6aC hzdw== X-Gm-Message-State: AOJu0YzJwQINkj3NXWjfmFG3sf/3XkETvVC9H6ejlFPdrz21jNb/pQYz gfsak1lhJTEhijMkI/9PyEoWA61RqIgGZw== X-Google-Smtp-Source: AGHT+IHv9vefuk1IiEciPmGKfj/kGXgVS9vckpjfG6jhYrnEZvagUm0jJUN3VJMGBLP0W9ZJcb89ww== X-Received: by 2002:a05:622a:1802:b0:415:141e:fa21 with SMTP id t2-20020a05622a180200b00415141efa21mr14374048qtc.6.1696290080615; Mon, 02 Oct 2023 16:41:20 -0700 (PDT) Received: from [192.168.1.35] (c-68-56-149-176.hsd1.mi.comcast.net. [68.56.149.176]) by smtp.gmail.com with ESMTPSA id jj11-20020a05622a740b00b004166ab2e509sm9229qtb.92.2023.10.02.16.41.20 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 02 Oct 2023 16:41:20 -0700 (PDT) Message-ID: Date: Mon, 2 Oct 2023 19:41:19 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US-large To: ffmpeg-devel@ffmpeg.org References: <20231002202523.148560-1-leo.izen@gmail.com> From: Leo Izen In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avcodec/jpegxl_parser: fix various memory issues 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 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 10/2/23 16:40, Andreas Rheinhardt wrote: > Leo Izen: >> The spec caps the prefix alphabet size to 32768 (i.e. 1 << 15) so we >> need to check for that and reject alphabets that are too large. > > No, we don't "need to", we can. FFmpeg is not a validator tool. We need to because we risk over-allocating otherwise. If the signalled value is far too large, we consume a pointlessly large amount of memory. > >> >> Additionally, there's no need to allocate buffers that are as large as >> the maximum alphabet size as these aren't stack-allocated, they're heap >> allocated and thus can be variable size. >> >> Added an overflow check as well, which fixes leaking the buffer, and >> capping the alphabet size fixes two potential overruns as well. >> >> Fixes: out of array access >> Fixes: 62089/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer- >> 5437089094959104.fuzz >> >> Found-by: continuous fuzzing process >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Found-by: Hardik Shah of Vehere (Dawn Treaders team) >> Co-authored-by: Michael Niedermayer >> Signed-off-by: Leo Izen >> --- >> libavcodec/jpegxl_parser.c | 23 +++++++++++++++++------ >> 1 file changed, 17 insertions(+), 6 deletions(-) >> >> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c >> index d25a1b6e1d..51af0f4ed1 100644 >> --- a/libavcodec/jpegxl_parser.c >> +++ b/libavcodec/jpegxl_parser.c >> @@ -46,6 +46,8 @@ >> #define JXL_FLAG_USE_LF_FRAME 32 >> #define JXL_FLAG_SKIP_ADAPTIVE_LF_SMOOTH 128 >> >> +#define MAX_PREFIX_ALPHABET_SIZE (1u << 15) >> + >> #define clog1p(x) (ff_log2(x) + !!(x)) >> #define unpack_signed(x) (((x) & 1 ? -(x)-1 : (x))/2) >> #define div_ceil(x, y) (((x) - 1) / (y) + 1) >> @@ -724,16 +726,17 @@ static int read_vlc_prefix(GetBitContext *gb, JXLEntropyDecoder *dec, JXLSymbolD >> if (ret < 0) >> goto end; >> >> - buf = av_calloc(1, 262148); // 32768 * 8 + 4 >> + buf = av_calloc(1, dist->alphabet_size * (2 * sizeof(int8_t) + sizeof(int16_t) + sizeof(uint32_t)) >> + + sizeof(uint32_t)); > > You can avoid the multiplication by using av_calloc((2 * sizeof(int8_t) > + sizeof(int16_t) + sizeof(uint32_t)) + sizeof(uint32_t), > dist->alphabet_size). That's not the same thing. This will cause us to overallocate by dist-alphabet_size - 4 bytes. Is that okay? > >> if (!buf) { >> ret = AVERROR(ENOMEM); >> goto end; >> } >> >> level2_lens = (int8_t *)buf; >> - level2_lens_s = (int8_t *)(buf + 32768); >> - level2_syms = (int16_t *)(buf + 65536); >> - level2_codecounts = (uint32_t *)(buf + 131072); >> + level2_lens_s = (int8_t *)(buf + dist->alphabet_size * sizeof(int8_t)); >> + level2_syms = (int16_t *)(buf + dist->alphabet_size * (2 * sizeof(int8_t))); >> + level2_codecounts = (uint32_t *)(buf + dist->alphabet_size * (2 * sizeof(int8_t) + sizeof(int16_t))); >> >> total_code = 0; >> for (int i = 0; i < dist->alphabet_size; i++) { >> @@ -742,6 +745,10 @@ static int read_vlc_prefix(GetBitContext *gb, JXLEntropyDecoder *dec, JXLSymbolD >> int extra = 3 + get_bits(gb, 2); >> if (repeat_count_prev) >> extra = 4 * (repeat_count_prev - 2) - repeat_count_prev + extra; >> + if (i + extra > dist->alphabet_size) { >> + ret = AVERROR_INVALIDDATA; >> + goto end; >> + } >> for (int j = 0; j < extra; j++) >> level2_lens[i + j] = prev; >> total_code += (32768 >> prev) * extra; >> @@ -772,8 +779,10 @@ static int read_vlc_prefix(GetBitContext *gb, JXLEntropyDecoder *dec, JXLSymbolD >> } >> } >> >> - if (total_code != 32768 && level2_codecounts[0] < dist->alphabet_size - 1) >> - return AVERROR_INVALIDDATA; >> + if (total_code != 32768 && level2_codecounts[0] < dist->alphabet_size - 1) { >> + ret = AVERROR_INVALIDDATA; >> + goto end; >> + } >> >> for (int i = 1; i < dist->alphabet_size + 1; i++) >> level2_codecounts[i] += level2_codecounts[i - 1]; >> @@ -848,6 +857,8 @@ static int read_distribution_bundle(GetBitContext *gb, JXLEntropyDecoder *dec, >> if (get_bits1(gb)) { >> int n = get_bits(gb, 4); >> dist->alphabet_size = 1 + (1 << n) + get_bitsz(gb, n); >> + if (dist->alphabet_size > MAX_PREFIX_ALPHABET_SIZE) >> + return AVERROR_INVALIDDATA; >> } else { >> dist->alphabet_size = 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". _______________________________________________ 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".