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 015CE41254 for ; Fri, 26 Aug 2022 16:01:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 72C6168B9F1; Fri, 26 Aug 2022 19:01:34 +0300 (EEST) Received: from EUR03-AM7-obe.outbound.protection.outlook.com (mail-am7eur03olkn2053.outbound.protection.outlook.com [40.92.59.53]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D242E68B9C7 for ; Fri, 26 Aug 2022 19:01:27 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oSC9LLVDgJ6bkV9cmlyWj8VKhkNv1dQAQ6SK0EOJPxem24QfXZpE4A8cxGBFgf0qm9IhNrlcAmeUGc6CuKrAVuC/Y+fJULfBtj6eatcFaWvKbd7hcExXl4tr9cs9BcnWVeTTbkeZiHlbpWpnxXKX85zRAief53ZICi48S7d7Um25+Zo68Xm0qWYRqZHNdy4OEXk1WzDfhcMiGo21gGbfWtWWDGqFtJimW7Bouo6qkgDvorXI5ky3k+LecTMPcULtVNT+qmCVUFfw4qSGqAdY0Ca2sveYubyP0KJOQw54nWMzdiKwQJZLXdZ4rauRJ7xwKN5JexqPZOIF83fJFXFxhA== 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=GWgEdA58uNh3MARUzJQwHBezDhVGwWfclfPWZHkdUhQ=; b=PGhMarwjWyhf+5Av2KkHtHlxAdNyy5bzTYqcrjG4Q5tQpOIw8abhnDQJR3SEQoQXcWOPP4yaNuuOP7Zenkd1SA7DXuW4KRb/s7RLv0lvCRuX4TK/wRNCWl4HxX76skaIKCYxg/bxJ73Xlvv0D0ljkzap30pJts17TOJQhSjq23TTZKpmf5SZW88vBmyZgG8Qe4L2pEi23fTmmUMRU4+Albe4LKEnS28d4XeXpexOYcBfFEsB8F/sdmD/2n78aXazxlYGcT7zoBNkk4ToLiYrFS5qzSN0URmT2eLs3nOiPPQDSuyoxtPJNjgTBKc83BAju1CitECWHa/JLLQbb22K+w== 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=GWgEdA58uNh3MARUzJQwHBezDhVGwWfclfPWZHkdUhQ=; b=cDBJpjgi2kv7zowePGVUpb3dzX9lcDpzL3hByZ5paa6QFjoXVDVmr5xrQYxWkVQbZ+jlGVi3/2Fjk/37gxxnO2tlVxBav9k+EKg8egn+uOua2TGqTrKr4LCObhPn0fumg8myxXl4/IPIr6woYKb5SeyhvMZjO0docimKSQp+AfKE4z+4lB5cvOrw9xIwS2fRTD0eais6Yv86BmgwUxWI0g8sNOUTPYQLlw94qhUw9s0IdWwW2u/g3o7bPnBCjJU3I0JUnGoJwHfi9iQbLMSf/ncZi9B68Gb91g2rpQog1CqcX816fjaVZRO2l3BCuXuDFD8YlL16zawsEyMi3BDYGA== Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) by AM0PR0102MB3090.eurprd01.prod.exchangelabs.com (2603:10a6:208:1d::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5546.21; Fri, 26 Aug 2022 16:01:25 +0000 Received: from DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46]) by DB6PR0101MB2214.eurprd01.prod.exchangelabs.com ([fe80::210e:b627:bcc9:8c46%11]) with mapi id 15.20.5546.026; Fri, 26 Aug 2022 16:01:25 +0000 Message-ID: Date: Fri, 26 Aug 2022 18:01:23 +0200 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220826064456.92895-1-lq@chinaffmpeg.org> From: Andreas Rheinhardt In-Reply-To: X-TMN: [JCqA+3NhRxavrvrahmSh0VJJqe21G9zb] X-ClientProxiedBy: ZR0P278CA0110.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:20::7) To DB6PR0101MB2214.eurprd01.prod.exchangelabs.com (2603:10a6:4:42::27) X-Microsoft-Original-Message-ID: <1f856fed-88b1-1be3-ac4b-3c0b1d6cdb09@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: b32c78f4-0b70-4a1a-c162-08da877c3a7f X-MS-TrafficTypeDiagnostic: AM0PR0102MB3090:EE_ X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Z0sXKN5RnsYyvdaAVSyWmbS72WRQhuIYhmE9iLYWvbGzadzKPT6rOGOxuWD1Q0phcSsxzWdIJKxAIQATAXlFcCSP7N8QRfaPe48zJCeMPISOXu0URfoHvw61/zBJxKq6v2iC4Dp6ntyEI5UPAw7u4u6Q2gBSRjtGUfDOUloC7GuypxIaJlmuaItehNALRohMcWx+vCDqL8hsjYNDwaoD5vsl7fD7HmiZPJQ+RHnsGXBmX+xGOrWUe/vEAJIwsr8rJwP95/GHFV8ehIMyC3PU5wCM/H2jNCynr4sCyYcIxPWCxlkQ2hgGTuhSnAgLDAV9Dxn6lBuP7HmCl3VsEJdtXOiBEQnQ+HAR1fZz2r8NSbIXhI6XXyBvmrNl/i3KUHmODJB60cZTpVlWf6QdEQLVKC93+Wos1ICEKA7Zk3qQ78DAk+FxXs1MxWPivn4sp5YX43DKjKGohWNyMF51ZzDWnDc39WNqNhpSBMnaiumd7NrmYKyJ8moB0mLdc+ax305O187usQKEThIeeKDgMv6rAkleA02wUPoRz9kvKDIL5oEJvjgPn33ZQ85ynAgqNFheSqQX52JRFVc30gU8L5CXyJkAY9hx5UqtIAhyI/lRT4eC0vBGapdHsGNmuORa38HHrobGAr8GLrVTW7HdEuU6T6aiKBVQp8Tq743XuJgHt5yooSGNs2mE4ArUO+ZF1GSc X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RVVMT3FKRVVvT2FEc2RjSXFkWkttYkM3SGpaSkpLYnVaRFdEcktQUE8vMCsy?= =?utf-8?B?RWhDcTA3bUhZdTV0T0ZBQ2ZwbUtodXNNSnZVR3h3Q1RxcEdpYlRDNStiWW8r?= =?utf-8?B?Tmgrb2s0TUZBenlLaVYvZW1XV3JzMk5LbHNpVEh5VWpHQ292MVljRjl5ZTdR?= =?utf-8?B?OXBiMGZGKytheXNVWnZyWnRXUnZHS0wyQTRuVk5iWGMxMFZTYUFPc0dEME1D?= =?utf-8?B?eTAreUtJUEZrQ1FGRUR4Y29UVjg3a0VHaityYXBLUnlaN0dmcm5oZmFONjVG?= =?utf-8?B?b2RoelRYZ0p6a3ZRNnFWYmJsS2VhdEFsbnIxZ1FYVnpJNlhnWDdndnVlYkpp?= =?utf-8?B?MGdQZGZIWXVlajVlRGlmRlB6d3NlMUlzVTFFRDNrRXlpM3VjZmZDbEZwUkxB?= =?utf-8?B?N05ZR1E2MVQxakVLVUxvR3RYZElaTDl6TVNXdHJ1QnhUWXpaUDd0eGd1RHox?= =?utf-8?B?ODQxY3I3NTNvVTB2VFM1eFF3dm11ZitvbzNweFRzU1NzQnlMV1hnVWhTRDRX?= =?utf-8?B?TXJHWUE2dVBGT1Q4RlRBelNwdWhCRllqakY5TVdWbTZ4Q3NvVVJYWU5na2Iv?= =?utf-8?B?RjdDUXVKTHRsdlFNY2VRbFNzZFJFNVJsZ0wxOGY4RGxFcVNwajNjVGRnUUdQ?= =?utf-8?B?VmFxTE1oZFNXYXN3eWFpSG13ODNkTTZzTDZxQUkyZmlJd09XVC85Um9hME1O?= =?utf-8?B?OTBMQjdnVjA0NTZxVExBMG42S2FZdldJMUhIdGZIOGlpa3RFQjBOc05IK09x?= =?utf-8?B?ZnQ3YUVBdFpEcXZVM2cvcDNOMXBkMENNSzQ2TFVrRW5Cb05KUlF2YlZSR2Qx?= =?utf-8?B?NjVMdHl4ZWFWK2JoOWJNRFgrVXhMekg2N1JkU0JGNXdwOVR1Vm5JTk93N3FT?= =?utf-8?B?dGRqb0ZLejhCUHZURVBlOFFiU0pRTVJzUjhnY1REc0RaVUNvcG41U29KaXQ4?= =?utf-8?B?N24rWldYRzNmdjVONEF6UTMyMENKb2w5NXdYUHdLSW5lNnA3U3BCZURJV0N6?= =?utf-8?B?a3VsVEVtZlorK3ZqSXJleGlpNktNQ293SEJqalhQRVJhbWFjbSswVW9Hd013?= =?utf-8?B?TzFOV3ZOd00yUHBqRms4Z2REWmdoc3FDa1U2OXR6Mktlejd4Yy9nOGlTckE2?= =?utf-8?B?dWhtWHlLUytPZjQ3bW5nWjFMOEQrd0svZU1sTm5QNVRyL3libmJLOXNJZThw?= =?utf-8?B?bDZEeTRKbkp4MldjNmwzbG90aFpmY1MrNDdReEVxWDdEOEhyK09HaEVnYXo0?= =?utf-8?B?S3RxUnlPMnhNTU5zekdvSktYb0libHlpS0hXbzhuVVZoL3o2WG9Qem5MQWt1?= =?utf-8?B?ZmlmR3plQ0tIMnZMclN5SWJ5YjRlM1phY2tXUDQ4RldwdG83SXZrNk5tZUpV?= =?utf-8?B?RHVmaHVZaWRKSGowakVPMytsd2hEbEpoNkxXNnh3Tkl2cWx6aGFmYVpZVjlT?= =?utf-8?B?Z2cvbzllc1prWXRCb2NBbk1UTkZ4bWdTZnQ0VTFXdXFNVUFtaHo5aG1vdkZh?= =?utf-8?B?N1YwRldqYzN6RGFIUGxCR2ljZXdVVTQ2cXhlL0todERyM0lrTDZLTGM3M0lF?= =?utf-8?B?UEIwMjBKay9XSll1VmJBR1djZ3FRRUdJWmwvVHhiMGhYbDB3QlZWRXoxNldF?= =?utf-8?B?eCtkUkhxL3A2bDkwbU1VMGRWY1AwQ1lwSzR2dzNONHpwcWlzeHdxM1pmc0sz?= =?utf-8?B?SkZhMWc5bi9iVFNoK3laNmVIcTVlbDZkVGRIMmc4QUgyQ3ZNeEpNODlFMHI1?= =?utf-8?Q?D9Lm5jRoPgRPbLZnCo=3D?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: b32c78f4-0b70-4a1a-c162-08da877c3a7f X-MS-Exchange-CrossTenant-AuthSource: DB6PR0101MB2214.eurprd01.prod.exchangelabs.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Aug 2022 16:01:25.7866 (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: AM0PR0102MB3090 Subject: Re: [FFmpeg-devel] [PATCH v2] avformat/imfdec: check track valid before use it 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: Pierre-Anthony Lemieux: > On Fri, Aug 26, 2022 at 1:37 AM Andreas Rheinhardt > wrote: >> >> Steven Liu: >>> fix CID: 1512414 >>> And return AVERROR_INVALIDDATA when get_next_track_with_minimum_timestamp >>> incorrect in imf_read_packet; >>> >>> Signed-off-by: Steven Liu >>> --- >>> libavformat/imfdec.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c >>> index 5bbe7a53f8..08f342bc1a 100644 >>> --- a/libavformat/imfdec.c >>> +++ b/libavformat/imfdec.c >>> @@ -697,8 +697,9 @@ static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVForma >>> } >>> } >>> >>> - av_log(s, AV_LOG_DEBUG, "Found next track to read: %d (timestamp: %lf / %lf)\n", >>> - track->index, av_q2d(track->current_timestamp), av_q2d(minimum_timestamp)); >>> + if (track) >>> + av_log(s, AV_LOG_DEBUG, "Found next track to read: %d (timestamp: %lf / %lf)\n", >>> + track->index, av_q2d(track->current_timestamp), av_q2d(minimum_timestamp)); >> >> Coverity actually complained about track being uninitialized, which this >> patch does not address. And the reason it does this is that it doesn't >> understand the algorithm: track will always be initialized in the first >> iteration of the loop. > > Is it possible to tell coverity that c->track_count > 0 is a > pre-condition, or should we modify the loop/algorithm? > The typical way to do this is to add an av_assert1 or av_assert2; but this must only be done if it is indeed ensured that the assert will not be triggered. >> (If there is a first iteration of the loop -- is >> this actually guaranteed? A file without tracks seems to be pretty useless.) > > imfdec currently assumes that (a) imf_read_packet() is not called if > there are no streams/tracks and (b) a track will always be found. > > (b) will be true for a conformant IMF Composition, but I am not sure > it can always be true for a malformed one. > Can't we make it true by adding the relevant checks to read_header? > I think imf_read_packet() can probably be hardened. Perhaps do this as > a patch separately from addressing the coverity issue? > >> FYI: In Coverity's analysis there are loop iterations, but it just >> assumed that track is not initialized in the loop (which boils down to >> saying that it presumed the tracks' current_timestamp to be invalid >> (denominator 0). I hope this can't happen. >> (There is btw another issue: The initialization of minimum_timestamp >> presumes that int are 32bit which need not be true.) > > INT32_MAX -> INT_MAX should fix this right? > Yes. >> >>> return track; >>> } >>> >>> @@ -760,6 +761,8 @@ static int imf_read_packet(AVFormatContext *s, AVPacket *pkt) >>> AVRational next_timestamp; >>> >>> track = get_next_track_with_minimum_timestamp(s); >>> + if (!track) >>> + return AVERROR_INVALIDDATA; >>> >>> ret = get_resource_context_for_timestamp(s, track, &resource); >>> if (ret) >> >> _______________________________________________ >> 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". _______________________________________________ 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".