From owner-svn-src-all@freebsd.org Thu Jan 18 01:43:01 2018 Return-Path: Delivered-To: svn-src-all@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 569CFE70835 for ; Thu, 18 Jan 2018 01:43:01 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx1.freebsd.org (Postfix) with SMTP id 3C57B6DF49 for ; Thu, 18 Jan 2018 01:43:00 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: (qmail 2610 invoked by uid 99); 18 Jan 2018 01:43:00 -0000 Received: from mail-relay.apache.org (HELO mail-relay.apache.org) (140.211.11.15) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 18 Jan 2018 01:43:00 +0000 Received: from [192.168.0.9] (unknown [186.80.205.98]) by mail-relay.apache.org (ASF Mail Server at mail-relay.apache.org) with ESMTPSA id D9D241A00E2; Thu, 18 Jan 2018 01:42:56 +0000 (UTC) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\)) Subject: Re: svn commit: r327675 - head/sys/netpfil/pf From: Pedro Giffuni In-Reply-To: <20180117233702.GR8113@FreeBSD.org> Date: Wed, 17 Jan 2018 20:42:52 -0500 Cc: Konstantin Belousov , Kristof Provost , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <6872E06D-738E-42C4-8D1A-C4381C21A269@FreeBSD.org> References: <201801071335.w07DZFWh069854@repo.freebsd.org> <20180107144423.GD1684@kib.kiev.ua> <20180117233702.GR8113@FreeBSD.org> To: Gleb Smirnoff X-Mailer: Apple Mail (2.3445.5.20) X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Jan 2018 01:43:01 -0000 > On Jan 17, 2018, at 18:37, Gleb Smirnoff wrote: >=20 > 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> >=20 > K> > Log: > K> > pf: Avoid integer overflow issues by using mallocarray() iso. = malloc() > K> > =20 > 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> > =20 > 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. >=20 > 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. >=20 FWIW, the associated NULL checks just became no-ops since overflows in = mallocarray(9) will now cause panics. Either the flags should be changed to M_NOWAIT or the NULL checks should = be removed. No idea which makes more sense. Pedro.