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 9CFC14A8E1 for ; Mon, 22 Apr 2024 23:52:42 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2CE4268D353; Tue, 23 Apr 2024 02:52:40 +0300 (EEST) Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9707068CC49 for ; Tue, 23 Apr 2024 02:52:33 +0300 (EEST) Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6eced6fd98aso4725575b3a.0 for ; Mon, 22 Apr 2024 16:52:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713829951; x=1714434751; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=oNBCLY31ngICPBxcELUSrt42i0P87wYghTJ8v+RhsP4=; b=HDpDRiZyAPb7M78z4+Mm5tACAsyR5JTzUoZ6t7nzDPO9KvTOAZQXke0XXYO/ao2EfZ Q/u4Tuc1DkDYVC0wQpbz14nupiOtnrIoIk2pjnn7n4j4gpEDJk7ugDq/1gdAAWkMZ39p cvUhPfMCtjNB0mSvBqcjuybaV0JhKFk0BEdfMmhUSGU4xodjPNjmkXxGvl0GeAGjdnsw AxQy1yfAst3TlxXOxQHHko8sU/lDxEzaIP05Ty4uw9VJjPzzzH9BeO79TsWHfZLLzA53 RFbUFh7Zke0AYoxiVr+ECGQ4odjSvqUJicxBIvid0o8FK/5nTSnoiYKuQP194RCqn9eJ FnOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713829951; x=1714434751; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=oNBCLY31ngICPBxcELUSrt42i0P87wYghTJ8v+RhsP4=; b=VAhPLQKmoHmtKeL/gipi3E8tokV7HpqwJpZRvklPjJxMoSIx0GKldoNI1vbRyBTmvs ZqYiViA03r2ywEXsmjopebAiJMjjmWVS41uDALyCCF+RdJpUffX+rZqytj6fkUeu2JVC 8QMvM6LzGwtW+jfJ+hHhvHNGLS6MzwRtqvS3vbqpTwDDYVzOWawmP9Oea+iWUDHN2SfW MQ0mxEYVJ55dE1p4FubkqIAIUr1wWT5vMm8lM5Rxmcw4vz+J4mcfBdtUlIzXFQKACGLR d2Msrl4Jps+eS2jTxqNR5mg01wt35+qWP4JEVnSYyy47YHa5M/nsnn238elDvK0kO5/Y Fy+w== X-Gm-Message-State: AOJu0Yyd60OCh/R3JQxYRwd9RB8ZbQWrUq9aTJ1sf2+2xJOBzYZ0WpnK hdb33mDqqaSfRshyBpQXBbTRP9ZJZhj3B5NzVp7uhnq0gAYftUPm+deJ7A== X-Google-Smtp-Source: AGHT+IFEB9DDbAuczrthGY6XivMPRG6WWrCrsjQ41v6zGF9iA1i3mgj4c3DBU1I9zk6c9myrqfxaog== X-Received: by 2002:a05:6a00:3d15:b0:6ed:21c0:986c with SMTP id lo21-20020a056a003d1500b006ed21c0986cmr19237295pfb.24.1713829950798; Mon, 22 Apr 2024 16:52:30 -0700 (PDT) Received: from [192.168.0.10] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id z17-20020aa78891000000b006ecf217a5e1sm8404526pfe.189.2024.04.22.16.52.29 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 Apr 2024 16:52:30 -0700 (PDT) Message-ID: Date: Mon, 22 Apr 2024 20:52:36 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240422013150.458103-1-michael@niedermayer.cc> <67b2f0ee-9465-4b76-ad15-be68d9faa987@jkqxz.net> <3ed83124-4f7e-4ed6-8e3c-3a97852617b7@gmail.com> <20240422210132.GD6420@pb2> <20240422234924.GE6420@pb2> Content-Language: en-US From: James Almer In-Reply-To: <20240422234924.GE6420@pb2> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/cbs_h2645: Check NAL space 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 4/22/2024 8:49 PM, Michael Niedermayer wrote: > On Mon, Apr 22, 2024 at 06:07:42PM -0300, James Almer wrote: >> On 4/22/2024 6:01 PM, Michael Niedermayer wrote: >>> On Mon, Apr 22, 2024 at 05:46:10PM -0300, James Almer wrote: >>>> On 4/22/2024 5:40 PM, Mark Thompson wrote: >>>>> On 22/04/2024 02:31, Michael Niedermayer wrote: >>>>>> Found-by-reviewing: CID1419833 Untrusted loop bound >>>>>> >>>>>> Sponsored-by: Sovereign Tech Fund >>>>>> Signed-off-by: Michael Niedermayer >>>>>> --- >>>>>> libavcodec/cbs_h2645.c | 4 ++++ >>>>>> 1 file changed, 4 insertions(+) >>>>>> >>>>>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c >>>>>> index fe2e383ff33..1a45d424bae 100644 >>>>>> --- a/libavcodec/cbs_h2645.c >>>>>> +++ b/libavcodec/cbs_h2645.c >>>>>> @@ -709,7 +709,11 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, >>>>>> start = bytestream2_tell(&gbc); >>>>>> for(i = 0; i < num_nalus; i++) { >>>>>> + if (bytestream2_get_bytes_left(&gbc) < 2) >>>>>> + return AVERROR_INVALIDDATA; >>>>>> size = bytestream2_get_be16(&gbc); >>>>>> + if (bytestream2_get_bytes_left(&gbc) < size) >>>>>> + return AVERROR_INVALIDDATA; >>>>>> bytestream2_skip(&gbc, size); >>>>>> } >>>>>> end = bytestream2_tell(&gbc); >>>>> >>>>> Seems fair. >>>>> >>>>> The problem looks more general with missing bounds checks in all the H.266 code around this, though? Compare with H.26[45], which have checks on all the reads - seems like H.266 should be doing that. >>>>> >>>>> Thanks, >>>> >>>> Not against this approach, but since the bytestream2_get_* functions return >>>> 0, never overread the buffer or move the internal pointer, wouldn't it be >>>> enough to just ensure end > start? >>>> Particularly in ff_h2645_packet_split(), we can return an error if length >>>> (in this case being set to end - start) is < 4. >>> >>> The patch adds the same kind of check as are used in the AV_CODEC_ID_HEVC >>> case earlier in the file already >>> >>> I think what is done should approximately stay in sync >> >> fwiw, better not add an initial length check to ff_h2645_packet_split() like >> i suggested, as it could potentially break samples that would otherwise >> decode just fine. It setting nb_nals to 0 should be enough. > > iam not 100% i parsed that text correctly I suggested to add an initial length check to ff_h2645_packet_split(), since it will do nothing (Other than set nb_nals to 0) if the buffer is less than 4 bytes long. If we do that, hypothetical bitstreams that for whatever reason pass a small buffer to ff_h2645_packet_split() and currently decode just fine will stop working. Just that. > > >> >> So this patch is fine. Further bytestream2_get_bytes_left() checks can be >> added too, namely one that checks the buffer is at least the smallest size a >> vvcC box can be. > > ok will apply patch > > thx > > [...] > > > _______________________________________________ > 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".