From patchwork Wed Apr 15 21:06:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 18988 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 15D4C447ADC for ; Thu, 16 Apr 2020 00:06:53 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F368068BB5F; Thu, 16 Apr 2020 00:06:52 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 32A5068B545 for ; Thu, 16 Apr 2020 00:06:47 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id t14so1864897wrw.12 for ; Wed, 15 Apr 2020 14:06:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=I2QSlHRCttsPBp08bca6w3HzJ5F45t0C4stC1f/DFLo=; b=s+Ms5ywLKGNAphi0ox9tYHIwexx+96Idm13cU1ZoIgEvTDXO4H9f2ffczuzZsMlWCJ zODrMIBwV81MZ0ZKbHjCPO+InvlzHXh6FKW5VBXeBXQ8dHdmoM8ZEVhP5OZuF5BCf2vl 6f2unvQia4/BNowtaA2JNz/U2GFyLEGl9z67ARI3d0qu1uLmDw3DiWLDBiafwMKTNA6e 80OhvbaM6ZM9kR5PMTxgOkzmelz76avCs1coGezgOnmha+ctra0gwSiayDbab0Got/8T AoHRrCwBO4QCuiNsaT/oFYpyfXY++N4ke36HUOzvAP0HkBhPQU6DtMXGM26X2JP2gYRK fnyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=I2QSlHRCttsPBp08bca6w3HzJ5F45t0C4stC1f/DFLo=; b=RlUY3q7ifbbDPzkcyoEbsDWVSmvEw3zkW4D72sYLOZJkfj1jWs4W7SSq7wW2+Ce+Ky Ji/YgOTlkW+BgZ9aOoaEfyMdQbrNVOEsrT+B3w9CNS96FL1eEBWVfzCXb+UbK1tNqOeg Xbi7/0DKYYnd983vfwGOZ2HRUEmBA+kXEdKLTObhbSufkJ1H+qUS+VK6HFkipY+XnyjN 8dsW7duatLsDByZNOQ2RwTG7UcwviqnIDw5Gvodm/cwbIEL5CbxJu2EaDG/KlJZpEBlX OEu3/+JDKjz97Y38DbQNNN116u66JJ215WwQHwtDo2ooRCkHU4wrasPtcpv9J58eYeuo WaRA== X-Gm-Message-State: AGi0Pua6bMkcUOQbpGWdAHyLRINgXzx4BdwB3CO7H8ATP1Z6JPxt17Op clk2gzKgaVqcAYV8RxZV1cyZR5Un X-Google-Smtp-Source: APiQypI4zb79Wi6FJOkqnOWM9hOFBytOnX4tKCpLW9MqFvUjtJKmo3N7MUHmaFlwpndWYWjAuwijwQ== X-Received: by 2002:a5d:420d:: with SMTP id n13mr31300970wrq.204.1586984806344; Wed, 15 Apr 2020 14:06:46 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1ab57.dynamic.kabel-deutschland.de. [188.193.171.87]) by smtp.gmail.com with ESMTPSA id f2sm24647605wro.59.2020.04.15.14.06.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Apr 2020 14:06:45 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 15 Apr 2020 23:06:14 +0200 Message-Id: <20200415210614.29152-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200415210614.29152-1-andreas.rheinhardt@gmail.com> References: <20200415210614.29152-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/2] avformat/matroskaenc: Make ebml_num_size() more robust X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Matroska (or actually EBML) uses variable-length numbers where only seven bits of every byte is usable for the length; the other bits encode the length of the variable-length number. So in order to find out how many bytes one needs to encode a given number one can use a loop like while (num >> 7 * bytes) bytes++; the Matroska muxer effectively did this. Yet it has a disadvantage: It is impossible for the result of a single right shift of an unsigned number with most significant bit set to be zero, because one can only shift by 0..(width - 1). On some architectures like x64 it is not even possible to do it with undefined right shifts in which case this leads to an infinite loop. This can be easily avoided by switching to a loop whose condition is (num >>= 7). The maximum value the so modified function can return is 10; any value > 8 is invalid and will now lead to an assert in put_ebml_num() or in start_ebml_master() (or actually in put_ebml_size_unknown()). Signed-off-by: Andreas Rheinhardt --- One can run into this infinite loop by adding an attachment with (int)size < 0 with ffmpeg on git master. If one applied this here without the previous commit, one would run into a well-deserved assert. libavformat/matroskaenc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index d3256d8f5d..407920172d 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -194,9 +194,11 @@ static void put_ebml_size_unknown(AVIOContext *pb, int bytes) */ static int ebml_num_size(uint64_t num) { - int bytes = 1; - while ((num + 1) >> bytes * 7) + int bytes = 0; + num++; + do { bytes++; + } while (num >>= 7); return bytes; }