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 4DC974A1C3 for ; Mon, 22 Apr 2024 21:07:48 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 14B6F68D383; Tue, 23 Apr 2024 00:07:46 +0300 (EEST) Received: from mail-pl1-f179.google.com (mail-pl1-f179.google.com [209.85.214.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7EFA268D135 for ; Tue, 23 Apr 2024 00:07:39 +0300 (EEST) Received: by mail-pl1-f179.google.com with SMTP id d9443c01a7336-1e455b630acso27583815ad.1 for ; Mon, 22 Apr 2024 14:07:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713820057; x=1714424857; 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=1IuQA6FHEIULPM9kQV+ARyARE/VEic16rnuk5QEyHY4=; b=XB5lekUtDci87zgvrA8ukjjrfX026pcl6I20VKEmKQHijy2Fvy+TVwxWLRpnUnHhEm hTLg6E+3FqHuHuu+pos3YdFMjdWhZk//M3k0CYnXKlzJaPVa5rrBMyanIadhrYvKaccY BaTtgTaBzvKgB3D3LgkXqO/tsY68wnQmBXXpj1bUS2L7oiXIQs1ci6tojqCwXoaSaUVa 7Sghwb7j6WQXWKAoqFKBXuJRr9EiIPvctk8wLVDmOxUpF20tPG3nwveiaS6C6cTlBBqa dn6LuvhMemc5t3hIiAmCYCZFqxXcbY40Q9H2U8tb6B3KnSWamww2PHX4sVBCZSQ36Ing v1BA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713820057; x=1714424857; 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=1IuQA6FHEIULPM9kQV+ARyARE/VEic16rnuk5QEyHY4=; b=HO7Bl1rQsktMqURS1AP3LpagvN4MiTb1V/r6ozXlEBSNXu06elwiqcT/0/R0DOGDF+ Lpl7VfbOsCjIs2v6Frl6Pvmc+mPoelfuOoMy6UlDSYSW0CF0S/V4ygU2O5uRd72w17fF UvcN91FVrmxlPTmsf3FytNhunwf4GVwR+t8UdsHVYBKC48NBOwwgssoievuMweAqHzmv OkiiZTO09/u2ExLmyVta9cQaBAW2CRpc/NQwpVKRm2AqMvf+c/IVx3/24YvXBTrnBFv2 zr4aJQxLyAtPmHCT6LVWT0dMhISnG5UqTwNaC8GOB0IrWFKjFZt6loW3FCiISytko8kw gsNg== X-Gm-Message-State: AOJu0YwuxsAhXFl6K78wt4/40MaA8esyTb9e7zr6v4zAQH3jQ8pkg2LF E/57cmq2IO6XV0APSyixT+ek2q1DKl3YHHrqscTpI1wviH2rHq/xjdEuSw== X-Google-Smtp-Source: AGHT+IE1f6Jr6kJRjEVIko4qX17i4xCRfI0f5+ls1b9/5z0DoQUZrlnba3ZxGQfGokVvjdGZHwBmJQ== X-Received: by 2002:a17:903:2289:b0:1e5:3c5:55a5 with SMTP id b9-20020a170903228900b001e503c555a5mr13246678plh.8.1713820056883; Mon, 22 Apr 2024 14:07:36 -0700 (PDT) Received: from [192.168.0.10] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id a5-20020a1709027d8500b001e3c77db2aesm8600156plm.88.2024.04.22.14.07.35 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 22 Apr 2024 14:07:36 -0700 (PDT) Message-ID: Date: Mon, 22 Apr 2024 18:07:42 -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> Content-Language: en-US From: James Almer In-Reply-To: <20240422210132.GD6420@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 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. 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. _______________________________________________ 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".