From patchwork Thu Jul 9 10:35:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 20912 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 C2B2944873B for ; Thu, 9 Jul 2020 14:33:27 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 98BAD68B5C3; Thu, 9 Jul 2020 14:33:27 +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 C4F8D689C71 for ; Thu, 9 Jul 2020 14:33:21 +0300 (EEST) Received: by mail-ej1-f68.google.com with SMTP id p20so1866648ejd.13 for ; Thu, 09 Jul 2020 04:33:21 -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:mime-version :content-transfer-encoding; bh=6edRKoW96giz6Ylon7sZGV/tG8V2ltgWeuQFQl31hGI=; b=gbFm+BvNGkIchsDC9Z0XHJKfaDujSqXPlD3B+0+3ZxTKxuFZ/eA1M+zb78ZO7/4CV8 KIr9rcCvBMErfqLjVr8pZCJjPxNIyDP6+sADZx+FyajTQnxsCzL/lJvwocLQgAV9AJZK kSH5TvaYdw6l3odvuh+i4KbmjvJQ/uhultQO0d7cbtvYLyGgZj3pm4k91GYbxnUN+qxe Z+TzZC+fZ6ISJijTrqgVqerojS0Lv4chyT1HCCOZcRogTQ13iw4NTHLYEU4/QtfyWT37 8x67j2yZ4JFgW+OZC4drdl2ymrFhPYgElhefbPP5exbNWG+xPr2TaI0GNB/zcUnO40RK TXgQ== 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:mime-version :content-transfer-encoding; bh=6edRKoW96giz6Ylon7sZGV/tG8V2ltgWeuQFQl31hGI=; b=DM9N6pCVBgfLkBLQMIKZeO8qd4UJQk4nc2vYPdKkWiR5cvOuo3Aev0f3gkMXgIGK2m 0kZz9jrx5kkQtvUZ9UM2ZwA/suSQVX9gbNsn8pD6+33pgUaH2OlTMAzRtZDcxM42anDE rqBmudGIn29fs5K+oxaJUfcY2CX9O7iPzpqSc7SgGx3qkUKbqfJJ3K+E3OPye1K02RW7 Atm8lFFVOJkhvbggl6p7smJiYo6qlbVAvWlXE7FDq+cPox7FeD6FkuuZTOFuHHGpL6oA TLqt1m5n5ZYFWyYrydKPCHQv7/QPF8AgGx/qDiN3IGyf+6OXBWX3u/fVHien/eOwrOFo PMPw== X-Gm-Message-State: AOAM530XE0K1hu+Cpc2w6TZLP7N7eF0Xbbdd/tDLPdbVwDskZOMF82oH B6pRYHz1v6NlS5qnrKSO2mg72foe X-Google-Smtp-Source: ABdhPJxxnVVVVYU03MuyimHU0Um88G955r+Ksn7a8pbDlnp+wXIR6n1jVdejc5Xnuh8f9x3DeOSfZw== X-Received: by 2002:adf:9286:: with SMTP id 6mr63046532wrn.361.1594290951352; Thu, 09 Jul 2020 03:35:51 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc10296.dynamic.kabel-deutschland.de. [188.193.2.150]) by smtp.gmail.com with ESMTPSA id c15sm4155623wme.23.2020.07.09.03.35.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jul 2020 03:35:50 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Thu, 9 Jul 2020 12:35:36 +0200 Message-Id: <20200709103542.19909-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/7] avformat/avc: Fix undefined shift and assert when reading exp-golomb num 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 current code for parsing unsigned exponential-golomb codes is not suitable for long codes: If there are enough leading zeroes, it left-shifts 1 by 32 places and uses get_bitsz to read 32 bits, although it is only suitable to read 0-25 bits. There is an av_assert2 to ensure this when using the uncached bitstream reader; with valid input one can still run into the assert and left-shift 1 by 31 which is undefined. This commit changes this. All valid data is parsed correctly; invalid data does no longer lead to undefined behaviour or to asserts. Parsing all valid data correctly also necessitated changing the return value to unsigned; also an intermediate variable used for parsing signed exponential-golomb codes needed to be made unsigned, too, in order to parse long signed codes correctly. Signed-off-by: Andreas Rheinhardt --- Moving the function to the beginning is done in preparation for another commit that I'll send soon. libavformat/avc.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/libavformat/avc.c b/libavformat/avc.c index b5e2921388..55494eb08a 100644 --- a/libavformat/avc.c +++ b/libavformat/avc.c @@ -27,6 +27,21 @@ #include "avc.h" #include "avio_internal.h" +static inline unsigned get_ue_golomb(GetBitContext *gb) +{ + int i; + for (i = 1; i <= 32; i++) { + if (get_bits_left(gb) < i) + return 0; + if (show_bits1(gb)) + break; + skip_bits1(gb); + } + if (i > 32) + return 0; + return get_bits_long(gb, i) - 1; +} + static const uint8_t *avc_find_startcode_internal(const uint8_t *p, const uint8_t *end) { const uint8_t *a = p + 4 - ((intptr_t)p & 3); @@ -318,15 +333,8 @@ static const AVRational avc_sample_aspect_ratio[17] = { { 2, 1 }, }; -static inline int get_ue_golomb(GetBitContext *gb) { - int i; - for (i = 0; i < 32 && !get_bits1(gb); i++) - ; - return get_bitsz(gb, i) + (1 << i) - 1; -} - static inline int get_se_golomb(GetBitContext *gb) { - int v = get_ue_golomb(gb) + 1; + unsigned v = get_ue_golomb(gb) + 1; int sign = -(v & 1); return ((v >> 1) ^ sign) - sign; }