From patchwork Thu Oct 8 17:57:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 22760 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 4DB51449AC1 for ; Thu, 8 Oct 2020 20:58:29 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2FAA968B919; Thu, 8 Oct 2020 20:58:29 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 2023768B845 for ; Thu, 8 Oct 2020 20:58:23 +0300 (EEST) Received: by mail-wr1-f65.google.com with SMTP id h7so7609008wre.4 for ; Thu, 08 Oct 2020 10:58:23 -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=7yy7IjQXgAsF8Q9WXCyYumiCrMiXiBm4kA+cYLx2bqI=; b=CoVquXqragBuW3N8oxdZ56VPCKkpMcAk0rMevUaVzfm2CLKy8y6mW6NnzbPJDzWAob fZ40qdpp916IjaAKeeM6LFBHBtF84IPSwlp/dBWmOMkeZndhJU2kuS5aQBuCWww2XYoG 8aS0h0NQ4MEvz2KQVdU7iSN6fKlyW3Leoj6iIQNywWKqAOBFv35LjAtJnPO8j5sDmnxO UWC2VaWZr4MjxBzkFXLfDb5eMX3XxVKn7wkpzAxWRkj07S1W4lJxrUG1eYPai5J2MsOA vnNbCEP82jtNOnAsxrUdFoSnEHS5Jtkl0Vt/VqQqNh3hpl8a6wkiK3eFiNLl1BAM9RqM sH3Q== 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=7yy7IjQXgAsF8Q9WXCyYumiCrMiXiBm4kA+cYLx2bqI=; b=nkP6xkkv+cJHcpslOvd0WAs0unVNJNAgy1+6sL0R2IIky5OAOuc1pxwr98l6X/LG/A 4LFdIeFHwVzAdTCmNTnlFdhAT3JVDwk247oeM6WbdXN11OoPaxtMvIR7l9NzDWVSSPhR u4sDwOEu7xahR2/Po1g9RLECAdp4nrADWq2rALoGUR6UFdOJFbeKXLv0a36q/YqsPhkD E0aNgwsEC5++FupoRTJLTBwwJKP+5jZ1EDk4k8plxvn2uzH1YXOkZTmfcxZVIhks4D5A 1pz9JZJB75DEu30RCbEUyYVaVt7a8Wi6vOt0pX/tray7Rx5DVxcHAbFPybLHyghJEmcM fJ6g== X-Gm-Message-State: AOAM530dUZVtU6Ge2R+YQS9kR3N3ymwqZcwyrLcbglKdTOMVN8a9YABW F0bWIAaGjlnC7oPwhiA5SjseobusBGA= X-Google-Smtp-Source: ABdhPJzvTI1W7AWenI4IXmSHUxVhEgGR4x2/BN0bEYQb3JsHUkA5Pd/UnyHatCax4SMuosyesNt8xw== X-Received: by 2002:adf:dd51:: with SMTP id u17mr10582384wrm.355.1602179902336; Thu, 08 Oct 2020 10:58:22 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc1aa4b.dynamic.kabel-deutschland.de. [188.193.170.75]) by smtp.gmail.com with ESMTPSA id h206sm8585374wmf.47.2020.10.08.10.58.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Oct 2020 10:58:21 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 8 Oct 2020 19:57:56 +0200 Message-Id: <20201008175756.290617-2-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201008175756.290617-1-andreas.rheinhardt@gmail.com> References: <20201008175756.290617-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: Use correct number of codes when init default VLCs 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" Commit bbc0d0c1fe2b7ecdc4367295594f084f85ad22f5 made the mjpeg decoder use default Huffman tables when none are given, yet when initializing the default Huffman tables, it did not use the correct number of entries of the arrays used to initialize the tables, but instead it used the biggest entry + 1 (as if it were a continuous array 0..biggest entry). This worked because the ff_init_vlc_sparse() (and its predecessors) always skipped entries with a length of zero and the length of the corresponding elements was always initialized to zero with only the sizes of the actually existing elements being set to a size > 0 lateron. Yet since commit 1249698e1b424cff8e77e6a83cfdbc9d11e01aa7 this is no longer so, as build_vlc() actually read the array containing the values itself. This implies that the wrong length now leads to a read beyond the end of the given array; this could lead to crashs (but usually doesn't); it is detectable by ASAN* and this commit fixes it. *: AddressSanitizer: global-buffer-overflow on address xy ... xy is located 0 bytes to the right of global variable 'avpriv_mjpeg_val_ac_luminance' Signed-off-by: Andreas Rheinhardt --- Hard to believe that this wrong number has not been found earlier. The code in question has been touched by about a dozen commits. libavcodec/mjpegdec.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 4128c47303..0a5ef110d1 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -96,27 +96,26 @@ static int init_default_huffman_tables(MJpegDecodeContext *s) int index; const uint8_t *bits; const uint8_t *values; - int codes; int length; } ht[] = { { 0, 0, avpriv_mjpeg_bits_dc_luminance, - avpriv_mjpeg_val_dc, 12, 12 }, + avpriv_mjpeg_val_dc, 12 }, { 0, 1, avpriv_mjpeg_bits_dc_chrominance, - avpriv_mjpeg_val_dc, 12, 12 }, + avpriv_mjpeg_val_dc, 12 }, { 1, 0, avpriv_mjpeg_bits_ac_luminance, - avpriv_mjpeg_val_ac_luminance, 251, 162 }, + avpriv_mjpeg_val_ac_luminance, 162 }, { 1, 1, avpriv_mjpeg_bits_ac_chrominance, - avpriv_mjpeg_val_ac_chrominance, 251, 162 }, + avpriv_mjpeg_val_ac_chrominance, 162 }, { 2, 0, avpriv_mjpeg_bits_ac_luminance, - avpriv_mjpeg_val_ac_luminance, 251, 162 }, + avpriv_mjpeg_val_ac_luminance, 162 }, { 2, 1, avpriv_mjpeg_bits_ac_chrominance, - avpriv_mjpeg_val_ac_chrominance, 251, 162 }, + avpriv_mjpeg_val_ac_chrominance, 162 }, }; int i, ret; for (i = 0; i < FF_ARRAY_ELEMS(ht); i++) { ret = build_vlc(&s->vlcs[ht[i].class][ht[i].index], - ht[i].bits, ht[i].values, ht[i].codes, + ht[i].bits, ht[i].values, ht[i].length, 0, ht[i].class == 1); if (ret < 0) return ret;