Date: Wed, 17 Jan 2018 15:37:02 -0800 From: Gleb Smirnoff <glebius@FreeBSD.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Kristof Provost <kp@FreeBSD.org>, 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> In-Reply-To: <20180107144423.GD1684@kib.kiev.ua> References: <201801071335.w07DZFWh069854@repo.freebsd.org> <20180107144423.GD1684@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180117233702.GR8113>