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 B524145C90 for ; Fri, 2 Jun 2023 00:27:51 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8B44768C2BB; Fri, 2 Jun 2023 03:27:47 +0300 (EEST) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-he1eur04olkn2052.outbound.protection.outlook.com [40.92.73.52]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7AC4868C213 for ; Fri, 2 Jun 2023 03:27:41 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LdjxEES5FsaU5utyQpLi4As4nnpByhG6MPuW2lp8eEH5fIpMTmSc6ZwrV43Kst/Dzm3E4GMh0VDgRiYLhPXRfge+EE8PLpsCGZm2sOvYG5Bgwy0OIiHktEvS9yptZ/AYQ0bObedUWtrb1QDxl8tz+6tBZMEsF1GdBvX3czCE1kepgr7X3O2l6lQofTswvspQgYzfi678ZJlfcKiexlB9MQef9UIqrel1osPhwkOA3Xl6AMEfCRfFZZdtA6J7xYDYQ0wiPctZ27ni0aGXmZBZVC3247crLe3g08AkO+ILC7J4I8kEuXBliEmO94xz6TAG2Laq95Fo27UxPNT3Dy6wnw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=NWKUb0z2Ka+V0nJNaCg8tNyn+sOY8I/l5vmvohySljA=; b=dh8Ver0ehcaTE4uL4EI+3x1gbAFdoFRjpzIqng+fTpyVS+m4w2faxMknJYm4rmxiprUaXmnbi6A8Bnom4DWydBnhc+t5X5pQ6azQ3mIVdubtL7zpD7KS1I8TyWYp6akMu5VMPBvT2bLK7NzVHLENUiK+lRe0NMnlR32CmSCiCqo/uP4ZpLav6y64yI1gLTTI5EV20GXa0Q7grC6Lri2YWrdhVtqeiMOLVI3YXZpyRv7FWJ4EGcHKDF4i/FXsb1nVcCoQ0zFGhbweyMT0E0Zh6XiV5S2cLld9YTxdwO3juP9uUOMDZcuFpsJbtyL9bXrDy1cmkUCcLV1xUM75x/MbbA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NWKUb0z2Ka+V0nJNaCg8tNyn+sOY8I/l5vmvohySljA=; b=D+EM488ndLNsg4D4Bm8CnKOzoRrvTW9wntRJLm/DsfCnd89C0b0Jh3DaifOBJhB46qqp4/vaCqFDBuPsjwqJhaXJwCaA3lTOZ0sDT/8Nn5M782rIPc2ukJdbvwuD6UgHm395fGWmllp+Sg4WwJL7QcT+1sKYunLkMfi4DC+LLMK5rMRx8hyxnBBJf0Q0fu+TbBNXq+k6URZWFtBhAWSGl2FvknnEisBPHUG4sMn2lLzJH9Rkc6QQjE7ZnnbLKn3hp0F9mKfBAB5/FlKAg3y9jEFiDJACmwAUMBqFgKQ2nkc0iE+nB91YTcH+jR/rmiH9M65CM41OjYJn0Qhrj59FCA== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by AM8P250MB0089.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:36c::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6455.22; Fri, 2 Jun 2023 00:27:38 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::ca17:cb7:b20d:3b65]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::ca17:cb7:b20d:3b65%5]) with mapi id 15.20.6433.020; Fri, 2 Jun 2023 00:27:38 +0000 Message-ID: Date: Fri, 2 Jun 2023 02:28:24 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20230601222601.12631-1-michael@niedermayer.cc> <20230601222601.12631-2-michael@niedermayer.cc> From: Andreas Rheinhardt In-Reply-To: <20230601222601.12631-2-michael@niedermayer.cc> X-TMN: [Lncq/q9uvR0rNLznrrS118VEt9wwJsQN] X-ClientProxiedBy: ZR2P278CA0082.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:65::11) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <13691410-2556-2747-7ba5-daf512c455de@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|AM8P250MB0089:EE_ X-MS-Office365-Filtering-Correlation-Id: 2f3dba9c-9822-4a40-60e7-08db63002b99 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: jb87ut5tAImqpc0nIfIcm/5dU0K1/103Z0W0syhlbvDaYMNNMVygPErPm7PIFLSZ+T2ZnwyUi5116IlbznEtMeZrK52Bqa9KDoW6EDTD3vbKcIpynryw7qbEyEsKfDHRfljxLEHcmQzOwkszumHCWl1+KP+WuA2V7gpUafiaFPXI7DT5Y96hxlMRwrriA4o/cy8k+y/vJmgJW27FdzpR23U5c/D+eoXGjLCXAgEyo6h4ktT2ucucAtVORDW34Xs24auhM+SEys2cXk1Jdg5ZWDSUOY+R1FzNzA/nqYA9KKikTbWRpVQP4elux+HJ91eBYkPWR5qxq+rAMPQtkkQB5xmOQnRoG79OkUe74FZeqUlAWIlIG7OVYOOTSncbN1YmpTkveN+ukewZZuYuujk6lV39MqPWz+CW3hOcnVeA5Zlou4HBFBnDyPK2+6luJrkLugluTFEckJGDOH5oSA98rn9W+059Bz0R9KVd2SvUheW+DCydx227fArNTCrFgOshZ9ns5CMehRmgRTZRmFaB0QVpalgHUAg5Er8LZlqFmzhiWdNz4CWjtifjUI9c7ed4nwGxJ5Mvvu5qXYh9auVQytvPewP8yqgppKuYf3QuHRQ= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UWxLVVM1MTdFYm96TFRkaGxkZ1FvQnFRdVU1TmJkYmx5N3Zoa3B3VEZiL2hK?= =?utf-8?B?bWg2TE5wcW1ldGZvdm9HNjhiSGpoWE8vRWJ4OWJaZStQbllFUHp4eVdIanRZ?= =?utf-8?B?ZUQ2Y1o4L1dsUExJNGRjTU0vL2hwcHJkZldsdHh6cTg4N2tGeDd1VXovYlZa?= =?utf-8?B?MHVIUHJnU2FNYmVDbkI3QXhQNVBETkhaVW9XdEZ2dWkwVVFObUNPbWJCeHcx?= =?utf-8?B?cWdXVWV4bnRwdU51azVsdUJ4KzNCZFVrV0sxRWpRQWdFOG41WU5SYzVRbk9D?= =?utf-8?B?L2RudHRxZVRmVFdBM0RDQjZDL2p3VENzVzM1cVllN2k2NG90eWxlb2JhbUox?= =?utf-8?B?N0hodTd2NENvQzU3eTd1SDdMck5qcXo3THhiMUVJcUg2ZDRVajlRYWdaVlB1?= =?utf-8?B?TWMzQnpyZzZhNjZaY2ZCNWVEeXBvMCtlNSsvVi9idk1UcDkzMzM2cHdsd3F4?= =?utf-8?B?Nlg5S0czdW8yandwOXMwTVU0SDlJVlpBa2plcUNuTUc5OG1TZm1XY1Y3aWxU?= =?utf-8?B?Z3pzNENPWEh4SXdmKzlkaXFNV2JxVldpMDhwdmlNeTZDYnVPaWcrMkRWL3I3?= =?utf-8?B?QXB0U1gzeGg4MWxRMUVEdTFqd1AxN2dpcFE1Qm1GMjFNMWRObkVUakR5Sm5D?= =?utf-8?B?RWNCMm5FRzNvcW9oRlo2ZkovM3lVNWs2WVlHcm9XM3hEdmRnNlJKUm9lblFG?= =?utf-8?B?Y3BxYThMeUgvMnZUWkx5K0pyNmpJTW5pNzc5UVdXRTI3d0w4TXZZc05Rd1pI?= =?utf-8?B?cnBwMnNVWmxCUk9BcmRDdUlmVi9ENEl5eWRHM1YyNW1kZTk0ZXdTTmVwS1Js?= =?utf-8?B?MlFKTzkvWE05aHlkU280dmtsWG1UNFo4QkJHWlIybW9YMVo3Q2hnK2hZWGxz?= =?utf-8?B?WHlKQUFydEtGa3p1QWllLzRyUHRjR0pKSmd3dlpWSjNEVTlsWERWVUJFalZD?= =?utf-8?B?RlJ2by9MSXI0alZDT1hnNG85QzhpaU14ZDc3RitnZjNPQ1VvY1VOOWV6cDdV?= =?utf-8?B?d3cxNVRNczhZSjBzNzJvQXBhUEp1bklHVHBpdkdQTDJRdi8zeWtGQTRhRTRU?= =?utf-8?B?TExlR3kzcDN1dDJWRVZOZFFGT3RhVUFjYnU1M21YaFpERmVZbU94Q2ExYUhp?= =?utf-8?B?UUYrWWNBTXRZeStiNmRSS1ZQZ0V5SzFnR3A0Sk54T0VZanIwS2svQ2NWUDRU?= =?utf-8?B?Q01DVGZERTNDb0kySk80TUc4cFBVRnF6WkJnajJ0NTFxTk1acFVsT1liemxV?= =?utf-8?B?K2ZYeG8rTzFTcVY0cEtvWmYrWEpTYzZUM3pQQkpWNk9QV2ZIa2JPakZVZGJp?= =?utf-8?B?YWFoVG1VRmVjNForMzRJWWlFQUc2Q0RueWFSRzhvclI1a2c3aFRwNUxrYVVL?= =?utf-8?B?d0IzZEszWHJ2UU9TWmZNZjdWbkFvOCs3Zkt6eEV1dmJvT0duMm1KTmxwMy9O?= =?utf-8?B?OVBSck95QUpQMGhUdlpJdE9TZFdIaGZmRk1sc3FaSGwxQllCR1RhdE1iV0M5?= =?utf-8?B?czVWdWVQMjY0ZVk0QzM5RXNsbHg2bUNlbXM5blVYZklEM2F6NHVzaVAwU3lV?= =?utf-8?B?cWh5bHdCUDhrVWpBTy94VllqOU9ZZ2FpZmF1L3dkTW9PUnlWdFVOK0dOdE5j?= =?utf-8?B?a0twc0U3M0tKNXdKbGpMaDA0NXJFcWVCNjBjSS91WDlDdEIySkV3c1hpV0lo?= =?utf-8?Q?6cRgyPOZFrc1vYWuzcpl?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2f3dba9c-9822-4a40-60e7-08db63002b99 X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Jun 2023 00:27:38.8716 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8P250MB0089 Subject: Re: [FFmpeg-devel] [PATCH 2/3] avcodec/cbs_av1: Clear obu.metadata on error 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: Michael Niedermayer: > On error pointers can be left NULL while code later assumes these not to be NULL > > Fixes: NULL pointer dereference > Fixes: 59359/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-6726080594313216 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/cbs_av1.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c > index 8788fee099..7f3f4da2f5 100644 > --- a/libavcodec/cbs_av1.c > +++ b/libavcodec/cbs_av1.c > @@ -1013,8 +1013,10 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx, > case AV1_OBU_METADATA: > { > err = cbs_av1_read_metadata_obu(ctx, &gbc, &obu->obu.metadata); > - if (err < 0) > + if (err < 0) { > + memset(&obu->obu.metadata, 0, sizeof(obu->obu.metadata)); > return err; > + } > } > break; > case AV1_OBU_PADDING: 1. Before 97f4263, the current_obu was reset (and the packet effectively discarded) upon errors from ff_cbs_read_packet(); yet this is no longer true and it seems that the contents of current_obu will be processed in the next call to av1_receive_frame(). This change seems to have been unintentional. 2. The commit message is weird: You claim that it is bad that a pointer is NULL; and then you go on and zero it again. It would be better to claim that the metadata is in an inconsistent state. 3. There is a possibility for inconsistency in cbs_av1_syntax_template, namely if the allocation fails, metadata OBU nevertheless claims to have a payload_size > 0; payload_size should only be set after the allocation succeeded. But this does not seem to be the issue in this testcase. Presumably there are not enough bits left for the itu_t_t35_country_code or its extension? In this case, cbs_av1 did not even make an error. 4. Your fix is dangerous: In case the code were changed so that an error can happen after a successful allocation, your memset would lead to leaks. (The most likely possibility is the addition of a new type of metadata; another way would be for the code to be changed to avoid reading the metadata twice by reallocating (and overallocating) the buffer as needed.) - Andreas _______________________________________________ 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".