From patchwork Fri Jul 31 11:22:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 21406 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 860B344998B for ; Fri, 31 Jul 2020 14:23:40 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 71E4A68B87C; Fri, 31 Jul 2020 14:23:40 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-ej1-f68.google.com (mail-ej1-f68.google.com [209.85.218.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1C11168B831 for ; Fri, 31 Jul 2020 14:23:31 +0300 (EEST) Received: by mail-ej1-f68.google.com with SMTP id o23so3606593ejr.1 for ; Fri, 31 Jul 2020 04:23:31 -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=UkSj3OwTnkL5d6tEvCghAetkKZB6nUNLNOKxdCXpeSc=; b=iUzZ9vhDCRHSJZ9VTlFLpEb8sAOCtRzQSzr5RqPFNdUrqZuVG7tyek82SZO46/mnJT fUmEw1yxxaDcQDfGDBQEeXMQE3h5g95zheAQpM3vVHn66IzN0jObguT7CMDq2rfbFvB3 nQugNRnSCOV/OW68cAc3EI15Rz/Wwvx9ybH2y1CdCzIpZ3Tp+zIyp8HdjzYQVHALucPh 7zGsj8RcJosTxURSuHhn2VbevGbUazaIJnP/UqwQtjDZPjHDzfNlZpsi4BPBftMGt63+ /jkEWeL4+FBQwZ+4Okp+x/vEWgZFJ8RgmKvCRj47ni/WZ692KufK4s7VXTxfd4QiaC3j XxGg== 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=UkSj3OwTnkL5d6tEvCghAetkKZB6nUNLNOKxdCXpeSc=; b=cqsP20VecEVt4WhUeD7IbCDRoShhinE7fP3RdHIyisxCRNTfZpp5/Lk5oKq3DJUZNK G8KOXpi9x4cYF/uFoTzH44C5JAUK5ExxsEp/qsv7ndR+a4OMPLWWmYsfBiBWpokcjFMU yjql6Al5LqxAIoJTE5bsxj5RvUO6xwcnky7keqin1Mi6cZm+JAdOegneXYFrsldB8lWN CRCc7sFcEYGyyKrHpjzGqiVkr4edRx9V8t3QDX06mmH8kK7wz2vGi/JrcuXQpZAFoe4+ 4DTIskR4pXH26Aw7eYYfHI89MJz5vjI2kyRfSGuGwnjHH0j8SxKNJGtdcZfCptjdSWLJ qRWw== X-Gm-Message-State: AOAM531xhyZT9ucHXMf2ng8ocdBgHdJntgJjeaOooyw8k68TihwE2u93 jZHYWDXJJU2UepALJXhlAzsNaNnz X-Google-Smtp-Source: ABdhPJyXm0h+wiusGvx/oD5CYYci4MsXEaizNNUMRIZyMWXv6Ui6tp6nOPT8szDUEOFSXa9i5ENMDw== X-Received: by 2002:a17:906:5f8a:: with SMTP id a10mr3678941eju.379.1596194610022; Fri, 31 Jul 2020 04:23:30 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id v22sm9284279edq.35.2020.07.31.04.23.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 31 Jul 2020 04:23:29 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 31 Jul 2020 13:22:38 +0200 Message-Id: <20200731112241.8948-5-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200731112241.8948-1-andreas.rheinhardt@gmail.com> References: <20200731112241.8948-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 5/8] avcodec/smacker: Improve header table error checks 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" The extradata for Smacker video contains Huffman trees as well as a field containing the size (in bytes) of said Huffman tree when stored as a table. Due to three special values the decoder allocates more than the size field indicates; yet when it parses the table it only errors out if the number of elements exceeds the number of allocated elements and not the number of elements as indicated by the size field. As a consequence, there might be less than three elements available at the end, so that another check for this is necessary. This commit changes this: It is always made sure that the three elements reserved to (potentially) use them to store the special values are not used to store ordinary tree entries. This allows to remove the extra check at the end. Signed-off-by: Andreas Rheinhardt --- libavcodec/smacker.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c index b6245a0ce1..8a4d88cfed 100644 --- a/libavcodec/smacker.c +++ b/libavcodec/smacker.c @@ -137,7 +137,7 @@ static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, return AVERROR_INVALIDDATA; } - if (hc->current + 1 >= hc->length) { + if (hc->current >= hc->length) { av_log(NULL, AV_LOG_ERROR, "Tree size exceeded!\n"); return AVERROR_INVALIDDATA; } @@ -244,9 +244,9 @@ static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext *gb, int ctx.recode2 = h[1].values; ctx.last = last; - huff.length = ((size + 3) >> 2) + 4; + huff.length = (size + 3) >> 2; huff.current = 0; - huff.values = av_mallocz_array(huff.length, sizeof(int)); + huff.values = av_mallocz_array(huff.length + 3, sizeof(huff.values[0])); if (!huff.values) { err = AVERROR(ENOMEM); goto error; @@ -259,12 +259,6 @@ static int smacker_decode_header_tree(SmackVContext *smk, GetBitContext *gb, int if(ctx.last[0] == -1) ctx.last[0] = huff.current++; if(ctx.last[1] == -1) ctx.last[1] = huff.current++; if(ctx.last[2] == -1) ctx.last[2] = huff.current++; - if (ctx.last[0] >= huff.length || - ctx.last[1] >= huff.length || - ctx.last[2] >= huff.length) { - av_log(smk->avctx, AV_LOG_ERROR, "Huffman codes out of range\n"); - err = AVERROR_INVALIDDATA; - } *recodes = huff.values;