svn commit: r327675 - head/sys/netpfil/pf

[Gleb Smirnoff]

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