From patchwork Wed Nov 9 19:56:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1364 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.90.1 with SMTP id o1csp473912vsb; Wed, 9 Nov 2016 11:56:10 -0800 (PST) X-Received: by 10.28.167.77 with SMTP id q74mr2100698wme.21.1478721370577; Wed, 09 Nov 2016 11:56:10 -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 r1si1890040wmf.122.2016.11.09.11.56.10; Wed, 09 Nov 2016 11:56:10 -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 EC43B689E04; Wed, 9 Nov 2016 21:56:04 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id EA11D689CAA for ; Wed, 9 Nov 2016 21:55:58 +0200 (EET) Received: by mail-wm0-f66.google.com with SMTP id c17so31302848wmc.3 for ; Wed, 09 Nov 2016 11:56:02 -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=h8oMdvzZYW7Sx1eq12IpyS5K28h4AAK3YYarnW6FHqw=; b=xIsdhY3epVSu7rBQxphFrciQHAuKEY9NV0kAMFsI1qppSRWQhiUptPNweB/LZ1iKui kqJbOByVE8WGCDsZgHDkHMjKnzoqvaLCsF1I6qDMu+bXC76WMLZ/00TKmZWUi/RaRK5G k0z7Yvan4LX1W6OKkZkJMY37NbTfD2XbffHT097utuHf/nEST/uxfBjnXdSoYsWcG3B7 +Dlt28hmdAfXYVEr9Jb8F6cdPcyNH/ouSkmPGTIiomJA4HI5YmWf/kGK8YwrFY8Xgx4V kOX40SkVJtbSmLdotUezjCmAI2ODFn1h3P0rOJyl1BLEfdDYARGqyTkDMO7UONdoQvgE ki0w== 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=h8oMdvzZYW7Sx1eq12IpyS5K28h4AAK3YYarnW6FHqw=; b=X2kZ0xdqnq7GFvlgousM1BYBPi1v20h3wSDz7ceuzsRrns/mZqcUnjH86rtpyfKQ5Q 9W11AEjQS3zuVHS6UHEDI1g+HK8ESr7c6vomPbaGXvqTclv2qNwk1cT0HCGmzBj9maQd JifbzhCd42c0LlmN0JgL2/1ZBRdDLgn/5uNJi5ESAMww0m2dpWlyKQ31NqdlU/5V4GeD COCFaZWn1ZIeOG+mWIMDNVp1txd28322O+aekZH0ybam8MOs1me3kmnkxzhYkiYWOSAc IpORo6mqlSnx8Uu+oshC+lERuTlsJ6vknsF2x4L3Fsk64fEuZP8T/wb12Qz8J4Z7wvtl GmnA== X-Gm-Message-State: ABUngvetArGtn5qjpvzSB2uFsvbHg7iP7Dbo+l0HpeIcIinKyFjG0kTeXYdsalSverejhg== X-Received: by 10.28.72.198 with SMTP id v189mr2242075wma.13.1478721361889; Wed, 09 Nov 2016 11:56:01 -0800 (PST) Received: from [192.168.2.21] (p5B072193.dip0.t-ipconnect.de. [91.7.33.147]) by smtp.googlemail.com with ESMTPSA id m5sm2030762wmd.17.2016.11.09.11.56.00 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 09 Nov 2016 11:56:01 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: <20161109013119.GA4602@nb4> Message-ID: Date: Wed, 9 Nov 2016 20:56:00 +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: <20161109013119.GA4602@nb4> Subject: Re: [FFmpeg-devel] [PATCH] icodec: correctly check avio_read return value 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 02:31, Michael Niedermayer wrote: > On Tue, Nov 08, 2016 at 11:36:58PM +0100, Andreas Cadhalpun wrote: >> It can read less than the requested amount, in which case buf contains >> uninitialized data, causing problems like segmentation faults later on. >> >> Signed-off-by: Andreas Cadhalpun >> --- >> libavformat/icodec.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/libavformat/icodec.c b/libavformat/icodec.c >> index 8019a35..aad1416 100644 >> --- a/libavformat/icodec.c >> +++ b/libavformat/icodec.c >> @@ -174,8 +174,8 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) >> bytestream_put_le16(&buf, 0); >> bytestream_put_le32(&buf, 0); >> >> - if ((ret = avio_read(pb, buf, image->size)) < 0) >> - return ret; >> + if ((ret = avio_read(pb, buf, image->size)) != image->size) >> + return ret < 0 ? ret : AVERROR_INVALIDDATA; > > is anything checking size to be positive ? > if not it could be matching an error code i think I've added a check to make sure that size is positive. New patch attached. Best regards, Andreas From 31f78a6d2906de4ee69e472aced5fef3af709b9a Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Tue, 8 Nov 2016 23:29:28 +0100 Subject: [PATCH] icodec: correctly check avio_read return value It can read less than the requested amount, in which case buf contains uninitialized data, causing problems like segmentation faults later on. Also make sure that image->size is positive, so that it can't match a negative error code. Signed-off-by: Andreas Cadhalpun --- libavformat/icodec.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libavformat/icodec.c b/libavformat/icodec.c index 8019a35..c273bf6 100644 --- a/libavformat/icodec.c +++ b/libavformat/icodec.c @@ -109,6 +109,10 @@ static int read_header(AVFormatContext *s) avio_skip(pb, 5); ico->images[i].size = avio_rl32(pb); + if (ico->images[i].size <= 0) { + av_log(s, AV_LOG_ERROR, "Invalid image size %d\n", ico->images[i].size); + return AVERROR_INVALIDDATA; + } ico->images[i].offset = avio_rl32(pb); if (avio_seek(pb, ico->images[i].offset, SEEK_SET) < 0) @@ -174,8 +178,8 @@ static int read_packet(AVFormatContext *s, AVPacket *pkt) bytestream_put_le16(&buf, 0); bytestream_put_le32(&buf, 0); - if ((ret = avio_read(pb, buf, image->size)) < 0) - return ret; + if ((ret = avio_read(pb, buf, image->size)) != image->size) + return ret < 0 ? ret : AVERROR_INVALIDDATA; st->codecpar->bits_per_coded_sample = AV_RL16(buf + 14); -- 2.10.2