From patchwork Tue Aug 1 23:25:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 4578 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.46.211 with SMTP id u202csp528671vsu; Tue, 1 Aug 2017 16:26:06 -0700 (PDT) X-Received: by 10.223.162.9 with SMTP id p9mr15575964wra.236.1501629966183; Tue, 01 Aug 2017 16:26:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501629966; cv=none; d=google.com; s=arc-20160816; b=bfgh/7OkNcBzRO0XBuq0q8U+OTdaWTevM6Fxk6YOLnrP7hf5K2ptFaaSnAI83fgKfS My/QYeN0737d08Fl6l4v/2WVUoC4DvIkuiNpzu4dMOhGwoJfGafynCgYb7KmTWE6b3eo ygtxikE+/OtvtJv5dqPruEFifEPe7NwzZWysT/kidZz8ayKLDJspYdDeTMJXiMeeteAp WfNzuN0e003Wpg9jUkBX1dTiJP1UlwDkH5OQvfJizCKMh5dBGnVRTTu/aDsg/bSQq9Z8 cKhlS2W8UnwepK3zJOivMQsR8zt6rx72+TlFhAPGV34X0PTVFGhVTHVs/Mvy/a6eekYu WZPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:reply-to:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:dkim-signature :delivered-to:arc-authentication-results; bh=PI0ORaDTPsVcrXo7YksU1RT7PRu6yWr4jeI40zlQJlA=; b=xnVtexKY2BHuUXiaguHDwr1xDR4NndgsQA4elYjG1Wf7XyPFE/bhodykPoJNhOKU3e GqKoMaFsQfTqQCd1yKVEAliyKbtUijFIKFEYiNAI+FPTh+7XSczWnsj00WTKohFQ1ztF P8MJ1VOHaYsg8/D7EZJPk8qNxdGxIfwpCLuWEGmpv/rDKYMRRmfqi4FYiOsqJ08krkFZ tUBq9weQEg6xfde1lImVTsJj7N2UEAyVMn8kseM1/NSMnAZ2DMXeoFmITT4z2K67NOJA sxrHuyGNmng842Y/tk45YCrrqzikFckw8yiaZSSk6kykx0V3NvGeBdAolOxRSvMONpbN JncA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.b=gWNqn3EV; 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=NONE sp=NONE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id q106si19749407wrb.291.2017.08.01.16.26.05; Tue, 01 Aug 2017 16:26:06 -0700 (PDT) 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=@gmail.com header.b=gWNqn3EV; 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=NONE sp=NONE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 62FE168A32E; Wed, 2 Aug 2017 02:25:59 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk0-f195.google.com (mail-qk0-f195.google.com [209.85.220.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E686468A288 for ; Wed, 2 Aug 2017 02:25:52 +0300 (EEST) Received: by mail-qk0-f195.google.com with SMTP id d145so3123297qkc.0 for ; Tue, 01 Aug 2017 16:25:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=qmRdtK2EcFXpdapIgTKzxkXBRlU1KsEOKUbLIyjDJw4=; b=gWNqn3EVXQ6L2oiIKWW0c26meTXVH19KlxFfYvu85r2nL0pG2/NBqTCmcZmg2S+2jz Knp88zw64LKa77BehPRjzE3lPxDJayCxhKsbERtQLxRKxy2+XtjFqn+vv44YPrDyb27h /3OzFMZ0+Q/opUkq1ioaawoyujAwwOeju4U5OtJSLJREw24dRiUwr8hq1uRowQFaz2W8 IuvORLVKdpZlsfHZnMpWVzIM0Iz1+k/Wq5l8DJLz9Hr2vfQ1Ca81yOX+NZQaF0xBnabE 3sbjbs8XmQUH1uCzgFtsyTj4coCtwDCvNtvAe2jwyS0wmgN91ys8t2ofUOgAJgga7GHv Y7Fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=qmRdtK2EcFXpdapIgTKzxkXBRlU1KsEOKUbLIyjDJw4=; b=uc/7pu7Olp/VpaegzrWqNI92DHm3vcY1MJTXfTLIP040/bqrxZ3SUgYIsfx9i8GzOa 20zHeGgIRNMhMAWYSYqegLCH4zhWVtmeEvDEGfGj8ho+keM8/+zilyu3zttNTdD60kWh q0nzp4jIeh7S0v0t9502Bh7JuhEQXYjhu/O6d2XooJN9CUfUtAVR9sxox4PjvqSTNkHF azRqztAWNNq40DcemodLfVZafb4WS3chunu03a//5cnDmjyFGxJn+WzU11dtDhPVRXiX 5oI5r9uFAtYk8TX3UyhBPaZMn2OSS/westD852vayIl4jGdcRRnJug5SGc/aRVbGdoZt C3iQ== X-Gm-Message-State: AIVw113mGcpWVgd2cp/M8v2UmPbWnu0Dg+gdf9Xhn7TXEZOMJzbYEslU wbNSpocxrWRPbcWZsuE= X-Received: by 10.55.198.132 with SMTP id s4mr26423993qkl.280.1501629954457; Tue, 01 Aug 2017 16:25:54 -0700 (PDT) Received: from [192.168.0.4] ([181.231.116.134]) by smtp.gmail.com with ESMTPSA id l67sm24686556qkh.34.2017.08.01.16.25.53 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 01 Aug 2017 16:25:53 -0700 (PDT) To: ffmpeg-devel@ffmpeg.org References: <1501569234-29896-1-git-send-email-rodger.combs@gmail.com> <1501569234-29896-6-git-send-email-rodger.combs@gmail.com> From: James Almer Message-ID: Date: Tue, 1 Aug 2017 20:25:50 -0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1501569234-29896-6-git-send-email-rodger.combs@gmail.com> Content-Language: en-US Subject: Re: [FFmpeg-devel] [PATCH 6/7] lavf/flacenc: avoid buffer overread with unexpected extradata sizes 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 8/1/2017 3:33 AM, Rodger Combs wrote: > --- > libavformat/flacenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c > index 9768b6a..1906aee 100644 > --- a/libavformat/flacenc.c > +++ b/libavformat/flacenc.c > @@ -322,7 +322,7 @@ static int flac_write_trailer(struct AVFormatContext *s) > if (!c->write_header || !streaminfo) > return 0; > > - if (pb->seekable & AVIO_SEEKABLE_NORMAL) { > + if (pb->seekable & AVIO_SEEKABLE_NORMAL && (c->streaminfo || s->streams[0]->codecpar->extradata_size == FLAC_STREAMINFO_SIZE)) { In what situation would stream extradata or c->streaminfo be smaller than FLAC_STREAMINFO_SIZE? Nothing changes par->extradata anywhere, and c->streaminfo is always allocated with FLAC_STREAMINFO_SIZE bytes of memory. I have the feeling i already asked this before, but not sure if you answered it. Apologies if you did. You can also simplify this by just rewriting the STREAMINFO header only if c->streaminfo is allocated, meaning updated extradata showed up in a packet. Otherwise, you'd be rewriting the same STREAMINFO header you already wrote at the beginning. You could also even use ff_flac_write_header(), which already does a buffer size check. avio_flush(pb); } else { diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c index b894f9ef61..7392cf13c1 100644 --- a/libavformat/flacenc.c +++ b/libavformat/flacenc.c @@ -141,17 +141,15 @@ static int flac_write_trailer(struct AVFormatContext *s) AVIOContext *pb = s->pb; int64_t file_size; FlacMuxerContext *c = s->priv_data; - uint8_t *streaminfo = c->streaminfo ? c->streaminfo : - s->streams[0]->codecpar->extradata; - if (!c->write_header || !streaminfo) + if (!c->write_header || !c->streaminfo) return 0; if (pb->seekable & AVIO_SEEKABLE_NORMAL) { /* rewrite the STREAMINFO header block data */ file_size = avio_tell(pb); - avio_seek(pb, 8, SEEK_SET); - avio_write(pb, streaminfo, FLAC_STREAMINFO_SIZE); + avio_seek(pb, 0, SEEK_SET); + ff_flac_write_header(pb, c->streaminfo, FLAC_STREAMINFO_SIZE, 0); avio_seek(pb, file_size, SEEK_SET);