From owner-svn-src-head@freebsd.org Wed Jan 17 23:37:04 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E2813EBF983; Wed, 17 Jan 2018 23:37:04 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (glebi.us [96.95.210.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "cell.glebi.us", Issuer "cell.glebi.us" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id C941F68639; Wed, 17 Jan 2018 23:37:04 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebi.us (localhost [127.0.0.1]) by cell.glebi.us (8.15.2/8.15.2) with ESMTPS id w0HNb3qJ017472 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 17 Jan 2018 15:37:03 -0800 (PST) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebi.us (8.15.2/8.15.2/Submit) id w0HNb2Wr017471; Wed, 17 Jan 2018 15:37:02 -0800 (PST) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebi.us: glebius set sender to glebius@FreeBSD.org using -f Date: Wed, 17 Jan 2018 15:37:02 -0800 From: Gleb Smirnoff To: Konstantin Belousov Cc: Kristof Provost , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r327675 - head/sys/netpfil/pf Message-ID: <20180117233702.GR8113@FreeBSD.org> References: <201801071335.w07DZFWh069854@repo.freebsd.org> <20180107144423.GD1684@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180107144423.GD1684@kib.kiev.ua> User-Agent: Mutt/1.9.1 (2017-09-22) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Jan 2018 23:37:05 -0000 On Sun, Jan 07, 2018 at 04:44:23PM +0200, Konstantin Belousov wrote: K> On Sun, Jan 07, 2018 at 01:35:15PM +0000, Kristof Provost wrote: K> > Author: kp K> > Date: Sun Jan 7 13:35:15 2018 K> > New Revision: 327675 K> > URL: https://svnweb.freebsd.org/changeset/base/327675 K> > K> > Log: K> > pf: Avoid integer overflow issues by using mallocarray() iso. malloc() K> > K> > pfioctl() handles several ioctl that takes variable length input, these K> > include: K> > - DIOCRADDTABLES K> > - DIOCRDELTABLES K> > - DIOCRGETTABLES K> > - DIOCRGETTSTATS K> > - DIOCRCLRTSTATS K> > - DIOCRSETTFLAGS K> > K> > All of them take a pfioc_table struct as input from userland. One of K> > its elements (pfrio_size) is used in a buffer length calculation. K> > The calculation contains an integer overflow which if triggered can lead K> > to out of bound reads and writes later on. K> So the size of the allocation is controlled directly from the userspace ? K> This is an easy DoS, and by itself is perhaps bigger issue than the overflow. Yes, this is one of the dirties parts of pf. The whole API to read and configure tables from the userland calls to be rewritten from scratch. Conversion from malloc to mallocarray really does nothing. Better just put a maximum value cap. -- Gleb Smirnoff