From patchwork Wed Nov 9 21:46:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1366 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp521407vsb; Wed, 9 Nov 2016 13:46:14 -0800 (PST) X-Received: by 10.28.69.217 with SMTP id l86mr4584441wmi.129.1478727974469; Wed, 09 Nov 2016 13:46:14 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id ul2si1681416wjb.158.2016.11.09.13.46.13; Wed, 09 Nov 2016 13:46:14 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E9CA3689E66; Wed, 9 Nov 2016 23:46:07 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1A0CE689E53 for ; Wed, 9 Nov 2016 23:46:01 +0200 (EET) Received: by mail-wm0-f68.google.com with SMTP id a20so21130880wme.2 for ; Wed, 09 Nov 2016 13:46:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to; bh=yNXOdVndbQKzyE91Irx8qK49bDB1UEjBb/ghggMi118=; b=Q4gy90bSCedZXUwB3SjcMwiloIKvATQKoqXARPji+6Jj5Fo+/LnoKKckHh0QxEPTQ7 AcP6FMei5yniCaCvBgwU+RlnHwd/+b+cbaQ2uOM4dwZ29x7iM4OTHHc9hmmKJU365APJ NGSptC2S/ozYTKB3zcd7JEfOHKNrFwalzVHK+ccMRrWx5b+6EcG1+6JpNQqXyNLC+KdM DZxLRWpGjgjBrs3ym7foZd7TF1q4OGzo/+GcK705seqrduJfGXABhijZc5ktTP0xdq1j fWbpJWzZnv76PzIKSoFacedxxuW5VWvyUNrrQPnKCPyj0tmTZKVBeYXnvtzd1UL9CXaQ 4ldQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=yNXOdVndbQKzyE91Irx8qK49bDB1UEjBb/ghggMi118=; b=QS/pgCVTwHrfKw/WM3HslAOaeySdcrvsTcN/jFLOsthz5nb94BGYpx4FboVroPgurB ACTuq41taT9+IaOWb+m35A+qCS0TVe/eE3KTf66DK1d67/DmzsBMZ5BmqoImOEe2n/hF DSW1L1ecRfvaHn9GCc5zUwQ147VA+rxZBa0SRAuFU+fg7W9MFdlQic9QYBbqYMUxDmTJ 9pWN/v1je/rvh8wdup/jeLwHY6ZmCOSqZ+LoYnq3EWFlDQmTer9M/Qg3nU45Qno59SXw CljTW++S7BXaa+R/xYXNZtqT2NSUkVu0lLwlTGHpSH5ktfD1i8JjsH599YsgctNcho7c nI7g== X-Gm-Message-State: ABUngved/7brJRZEhgYqvp2YtHoYhXCNIkEDzzNfyxBH/Pq7CUFHuwuSb5nBCgrpm1+TcQ== X-Received: by 10.194.158.100 with SMTP id wt4mr1855478wjb.148.1478727964897; Wed, 09 Nov 2016 13:46:04 -0800 (PST) Received: from [192.168.2.21] (p5B072193.dip0.t-ipconnect.de. [91.7.33.147]) by smtp.googlemail.com with ESMTPSA id u81sm1380151wmu.10.2016.11.09.13.46.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Nov 2016 13:46:04 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: <665e0d12-f81d-3e6a-6f4a-2feccf1b20ec@googlemail.com> <20161109101023.GD4602@nb4> <6880ed2d-64e0-3a28-9f67-06d05cb68f14@googlemail.com> <20161109205546.GI4602@nb4> Message-ID: <3b754390-2fc5-5594-54f3-2e2bd4d95f08@googlemail.com> Date: Wed, 9 Nov 2016 22:46:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161109205546.GI4602@nb4> Subject: Re: [FFmpeg-devel] [PATCH] pnmdec: make sure v is capped by maxval 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" On 09.11.2016 21:55, Michael Niedermayer wrote: > On Wed, Nov 09, 2016 at 09:05:17PM +0100, Andreas Cadhalpun wrote: >> On 09.11.2016 11:10, Michael Niedermayer wrote: >>> On Wed, Nov 09, 2016 at 01:11:29AM +0100, Andreas Cadhalpun wrote: >>>> Otherwise put_bits can be called with a value that doesn't fit in the >>>> sample_len, causing an assertion failure. >>>> --- >>>> libavcodec/pnmdec.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c >>>> index ca97cc3..0381ea6 100644 >>>> --- a/libavcodec/pnmdec.c >>>> +++ b/libavcodec/pnmdec.c >>>> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, >>>> /* read a sequence of digits */ >>>> do { >>>> v = 10*v + c; >>>> + if (v > s->maxval) { >>>> + av_log(avctx, AV_LOG_ERROR, "value %d larger than maxval %d\n", v, s->maxval); >>>> + return AVERROR_INVALIDDATA; >>>> + } >>> >>> indention is a bit noisy >> >> Fixed. >> >>> i think it can overflow if maxval is large, >> >> I've added an explicit check for v < 0, which should catch that. >> >>> it would be faster to check outside the loop >> >> However, such a check could pass if v overflowed so much that it's >> in the valid range again, so I'd rather not do that. >> >> Updated patch is attached. >> >> Best regards, >> Andreas >> > >> pnmdec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> 9b84227c054610b73977e3acde32734a47e46c0c 0001-pnmdec-make-sure-v-is-capped-by-maxval.patch >> From 7e9dcbde04ad95fc93ac2f0e91d734c8187c8d2b Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun >> Date: Wed, 9 Nov 2016 01:09:35 +0100 >> Subject: [PATCH] pnmdec: make sure v is capped by maxval >> >> Otherwise put_bits can be called with a value that doesn't fit in the >> sample_len, causing an assertion failure. >> --- >> libavcodec/pnmdec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c >> index ca97cc3..8961310 100644 >> --- a/libavcodec/pnmdec.c >> +++ b/libavcodec/pnmdec.c >> @@ -145,6 +145,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, >> /* read a sequence of digits */ >> do { >> v = 10*v + c; >> + if (v < 0 || v > s->maxval) { > > v < 0 implies v is signed > if 10*v overflows you have undefined behavior Alright, let's use a more complex check. See attached patch. Best regards, Andreas From f315a3cfe35377a2638dc2294200a288408dc784 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Wed, 9 Nov 2016 01:09:35 +0100 Subject: [PATCH] pnmdec: make sure v is capped by maxval Otherwise put_bits can be called with a value that doesn't fit in the sample_len, causing an assertion failure. --- libavcodec/pnmdec.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c index ca97cc3..0f6a895 100644 --- a/libavcodec/pnmdec.c +++ b/libavcodec/pnmdec.c @@ -144,6 +144,10 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, } else { /* read a sequence of digits */ do { + if (v > (INT_MAX - c) / 10 || 10 * v + c > s->maxval) { + av_log(avctx, AV_LOG_ERROR, "value 10 * %d + %d larger than maxval %d\n", v, c, s->maxval); + return AVERROR_INVALIDDATA; + } v = 10*v + c; c = (*s->bytestream++) - '0'; } while (c <= 9); -- 2.10.2