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 A9D0146913 for ; Sat, 24 Jun 2023 20:14:08 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F3C8268C15F; Sat, 24 Jun 2023 23:14:05 +0300 (EEST) Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F057668BF1A for ; Sat, 24 Jun 2023 23:13:59 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.de; s=s31663417; t=1687637639; x=1688242439; i=reimar.doeffinger@gmx.de; bh=XptNVtqkGeToyC5y+gSEVkGbEUEP+zmOFsZyn7gIr04=; h=X-UI-Sender-Class:From:Subject:Date:References:To:In-Reply-To; b=ob8PKBdXRwm4w1JtU2ihVQhShOqFXTnMEhcD2a4U3ndlUvF5ZmcNwcod05x5piKju9Q5UWz jnG4P6Nvd1hW+gE1VXyVqbCh8SLvwzdvv5hfEoxnqjSxAqT1AynjIPXTOjGAjhMGjth1CSdSJ dLLVvKjiI5K0Edatm3OZX3BcB/ViDK3vLz2TTUrh6+ey2kVa/Sgq/aTRNGFv3L6fFjwHYFGul OTfXscEZ8YujYIjSeVM/5Z87HFA8K6FxqSTP/qaccd+OqBPjX3N69lCIIHdgGRE6kKRTW29j8 KZbycskF9tgha16ldF0YmQp+HaT+EhaCKVAQevyu5Pq35vFDbEOQ== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from smtpclient.apple ([90.230.81.36]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MAOJP-1qNLzg0YZf-00BqIP for ; Sat, 24 Jun 2023 22:13:59 +0200 From: =?utf-8?Q?Reimar_D=C3=B6ffinger?= Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\)) Date: Sat, 24 Jun 2023 22:13:48 +0200 References: <20230622003038.20969-1-michael@niedermayer.cc> To: FFmpeg development discussions and patches In-Reply-To: Message-Id: <626B6C70-0258-444F-9AC3-6F83E539576D@gmx.de> X-Mailer: Apple Mail (2.3731.600.7) X-Provags-ID: V03:K1:4bnWeaIAPOZbgsX8JoXVZkcNA9fRVyzg8TjMly7c9G4e80U43kY 6f3utYA2P3cXMT5uFrIC//mmiHC/ehcv1jK68qz5R+VdWt6dLK1wI/Bpt1uNppuGaoUkd9W iLEIM9qrDkbPW1wcH6Ww3ojAKTbVl2Yv5c2p9vVw6+VcZRUJd9c0LdFrSJvN63VtbkLFsbb HaiERctdhkzfGnZvtDESg== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:N/GQZBmBDOM=;5TyMRvqry3W3uJXcU7x9cpWBqeM RvxxTDketq6lGRMk72HEK6ypM1fG4/lbqA73qZS9ZSzy+XR+QoQKiJKt8kBaXiN/CqBeqpNFh nawT5V4+FN1h++KkvngfemXUu6F1XDd6jMO/XcEjv4/CmemRD5FzIyyEWeeaHKnAUGHwkizNE FOJy3QdGHfGJmnb3yLwil1QaTyPhbVPrmFF7gbPN5eeX0HQHsCT1Z4jaEpe8mVaWqv7W87oVl W99yvig/9zuWMwcVnPWKcWPG77Eqrq2HeLNz9nLJzSDAdkFsKFvPF6NWz/WVV9+9IkKrG9h9q BnZAZhU/QTPq3kWVR2PCp7K7X/UpHXNDoZOuROoeBxI1dwSUwP7ulRxMoUVUKzcalEqfB4YkP s2uVTVWvxbI+EMPdiAd0G1oLVfGK/M6cpJDE631TBUSR5SkxoGMWwLKG8QaqhPpJmgKo5Rkio 1CwrO+l9cfyVuH2LoPqFzrVFAh2mXdvNHGx5LkWKLHQxlBv36oevTdHpi8tdpKacpTtw+ctjt VIkj6sofJBquAfudpWYx1dGAsnWzRdbtNHA1PBz4s8KMejHIjWxHtLf1qMkuq8b3DfG9hb1H/ RKcr9mOrgUdpQbCXBzUSGhIzx2UNHxk+mb3qPT5C2LMXsKmtD/rBAafGzXJpFu9tagTPWjU2j kac6rEFfe5u61MF4q+4HbXdy2lBPuMJWKUSoohCjqaBxESUiqoBrC9XPREREbZR3cZUP0jwjZ BRRNG7KufnZ8aLbYgN7T3J5o9fpxpzvvPrzrlDeFY0xfNIuTeui80kEaxY+eD0TZUJtd/iJe2 SxhY/MbYHOFPfbgLcg0rgbL4TXN2tx7QBV8dm1PU9I0NTH3xcOi8tQytA1bsxn7Wudq8m8Zae 9OjaAEmfgjH0wWdmZ4XShgas9CTOyZ8jRFch01/0IxGrySZyVt1Mrvi1t2duj3aHzIdfEtITr LPwX9g== Subject: Re: [FFmpeg-devel] [PATCH] avcodec/parser: Check next against buffer index 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-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: Hi! > On 24 Jun 2023, at 21:14, Andreas Rheinhardt wrote: > > Michael Niedermayer: >> Fixes: out of array access >> Fixes: crash-0d640731c7da52415670eb47a2af701cbe2e1a3b >> >> Found-by: Catena cyber >> Signed-off-by: Michael Niedermayer >> --- >> libavcodec/parser.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavcodec/parser.c b/libavcodec/parser.c >> index efc28b8918..db39e698ab 100644 >> --- a/libavcodec/parser.c >> +++ b/libavcodec/parser.c >> @@ -214,7 +214,7 @@ int ff_combine_frame(ParseContext *pc, int next, >> for (; pc->overread > 0; pc->overread--) >> pc->buffer[pc->index++] = pc->buffer[pc->overread_index++]; >> >> - if (next > *buf_size) >> + if (next > *buf_size || (next < -pc->index && next != END_NOT_FOUND)) >> return AVERROR(EINVAL); >> >> /* flush remaining if EOF */ > > Could you provide more details about this? E.g. which parser is this > about at all? And how can we actually come in this situation at all? > (Whenever I looked at ff_combine_frame() I do not really understand what > its invariants are supposed to be.) Yeah, when I looked at it I also felt like it has all kinds of assumptions/preconditions without which it will break, but those are not documented. Not really reviewable for correctness. The change I proposed to fix the same issue was as below. But note that it is not based on actual understanding, just that generally index, overread_index and buf_size are updated together so I thought it suspicious buf_size was not updated on realloc failure. --- a/libavcodec/parser.c +++ b/libavcodec/parser.c @@ -252,6 +252,7 @@ int ff_combine_frame(ParseContext *pc, int next, AV_INPUT_BUFFER_PADDING_SIZE); if (!new_buffer) { av_log(NULL, AV_LOG_ERROR, "Failed to reallocate parser buffer to %d\n", next + pc->index + AV_INPUT_BUFFER_PADDING_SIZE); + *buf_size = pc->overread_index = pc->index = 0; return AVERROR(ENOMEM); _______________________________________________ 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".