From patchwork Thu Nov 10 19:52:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1374 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp1031523vsb; Thu, 10 Nov 2016 11:52:44 -0800 (PST) X-Received: by 10.28.66.218 with SMTP id k87mr8067436wmi.79.1478807564445; Thu, 10 Nov 2016 11:52:44 -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 dy10si6903497wjb.80.2016.11.10.11.52.40; Thu, 10 Nov 2016 11:52:44 -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 3363D689E39; Thu, 10 Nov 2016 21:52:35 +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 2243F689E0A for ; Thu, 10 Nov 2016 21:52:28 +0200 (EET) Received: by mail-wm0-f68.google.com with SMTP id g23so5200730wme.1 for ; Thu, 10 Nov 2016 11:52:31 -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=35uDxcn0qpkag6qplBcpRRTgzcCvMeEz+tVlSMRHAuU=; b=e4dSKpZP2TqV71o41N3RHp2/t8HRBPf5CNIyL537ZPO9KcISU9gDfmxe1Lg226tz8R y9SwJp0nmlgorpalD2FqnvTuK55Wd8BFtGvrIQ079NP0f1xJyvrB+mtS8njwDcmc4G5m 6YjNJ1acUBDeryIsJI3lLIzdGbRic50cFxBO/ZcjLjTMRjg4EZVuBFDW40SAMkfqWKkR XFsFNgxRUWlTz5TLxW6xGnn2Anoq4pbzjrI+d1fd3rhw1k+cmtxU1fEC7PKLVN6/Qgf9 5+M+31oqr4c/ZPGis/9sOf4sKDSPAOOYV4dj+IQP4S2Ckor2Gkh+XjJbls23/pPVVxsA KuZg== 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=35uDxcn0qpkag6qplBcpRRTgzcCvMeEz+tVlSMRHAuU=; b=HsPPvFAJU7aFSPC+2OeVAPo8gn7iS2uws9TQYg9AqIUwZmJqJGIh1gXpqdUx6Furc/ jNTeQYyvVPyPiYlDQWvnho6i7JVanFipjlsuR2aPf3dcKKxGZM1IvfGit6RUXUXJRvG4 875sDecj2mgLFhOR+G6lgHwqrBzy/y+JfCFkreEHOhEYwUP68GFXJ7XPa19104jszSQY QWt0jhmodMe8KFluYU31D/LGRm02kgxHPyyNDGV+oLPB4RTRzK0zwXSoj2JWkG18n2r3 y0XJfqlXT2SJF8bXbZg6p26aBxmJrjOGVe5jJmAdU3bjeua/BuHZC/j1Y0HWHsQlXcPE tV2g== X-Gm-Message-State: ABUngvcxAjGbwVnoDEp9G2RbyAVnfNSnPeT5x7+R8u5ug0RZdFCvvZNM0ze2sbjQMwFzJQ== X-Received: by 10.28.191.87 with SMTP id p84mr7423032wmf.63.1478807551091; Thu, 10 Nov 2016 11:52:31 -0800 (PST) Received: from [192.168.2.21] (pD9E8ED50.dip0.t-ipconnect.de. [217.232.237.80]) by smtp.googlemail.com with ESMTPSA id hy10sm7252962wjb.10.2016.11.10.11.52.30 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 10 Nov 2016 11:52:30 -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> <3b754390-2fc5-5594-54f3-2e2bd4d95f08@googlemail.com> <20161110012644.GJ4602@nb4> Message-ID: <7fb0049c-c650-9f1a-bf9d-77b5d40e61af@googlemail.com> Date: Thu, 10 Nov 2016 20:52:29 +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: <20161110012644.GJ4602@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 10.11.2016 02:26, Michael Niedermayer wrote: > On Wed, Nov 09, 2016 at 10:46:03PM +0100, Andreas Cadhalpun wrote: >> pnmdec.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> a970cb981be02ea692d0bf2e68976077f14f2de3 0001-pnmdec-make-sure-v-is-capped-by-maxval.patch >> 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; >> + } > > this test should nt be inside the loop > you can try to benchmark with START/STOP_TIMER to see what effect > this has (i didnt try but i expect it to be bad), this is the > innermost loop of the decoder > > you can just unroll the loop by 5, thats the max number of iterations > i think, which avoids the overflow > it also should make the code faster Done, new patch attached. > iam reading in man ppm and others: > "The maximum color value (Maxval), again in ASCII decimal. Must be less than 65536" Indeed, I'll send a separate match limiting maxval. Best regards, Andreas From 2d402d8a38223b8c3c58d3e71e734932c9bdadae 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 | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libavcodec/pnmdec.c b/libavcodec/pnmdec.c index ca97cc3..958c5e4 100644 --- a/libavcodec/pnmdec.c +++ b/libavcodec/pnmdec.c @@ -43,7 +43,7 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, int buf_size = avpkt->size; PNMContext * const s = avctx->priv_data; AVFrame * const p = data; - int i, j, n, linesize, h, upgrade = 0, is_mono = 0; + int i, j, k, n, linesize, h, upgrade = 0, is_mono = 0; unsigned char *ptr; int components, sample_len, ret; @@ -143,10 +143,14 @@ static int pnm_decode_frame(AVCodecContext *avctx, void *data, v = (*s->bytestream++)&1; } else { /* read a sequence of digits */ - do { + for (k = 0; k < 5 && c <= 9; k += 1) { v = 10*v + c; c = (*s->bytestream++) - '0'; - } while (c <= 9); + } + if (v > s->maxval) { + av_log(avctx, AV_LOG_ERROR, "value %d larger than maxval %d\n", v, s->maxval); + return AVERROR_INVALIDDATA; + } } if (sample_len == 16) { ((uint16_t*)ptr)[j] = (((1<maxval>>1))/s->maxval; -- 2.10.2