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

[Adrian Chadd]

.. what's capping totlen so one doesn't run out of memory?


-a


On 24 February 2016 at 23:33, Kristof Provost <kp at freebsd.org> wrote:
> Author: kp
> Date: Thu Feb 25 07:33:59 2016
> New Revision: 296025
> URL: https://svnweb.freebsd.org/changeset/base/296025
>
> Log:
>   pf: Fix possible out-of-bounds write
>
>   In the DIOCRSETADDRS ioctl() handler we allocate a table for struct pfr_addrs,
>   which is processed in pfr_set_addrs(). At the users request we also provide
>   feedback on the deleted addresses, by storing them after the new list
>   ('bcopy(&ad, addr + size + i, sizeof(ad));' in pfr_set_addrs()).
>
>   This means we write outside the bounds of the buffer we've just allocated.
>   We need to look at pfrio_size2 instead (i.e. the size the user reserved for our
>   feedback). That'd allow a malicious user to specify a smaller pfrio_size2 than
>   pfrio_size though, in which case we'd still read outside of the allocated
>   buffer. Instead we allocate the largest of the two values.
>
>   Reported By:  Paul J Murphy <paul at inetstat.net>
>   PR:           207463
>   MFC after:    5 days
>   Differential Revision:        https://reviews.freebsd.org/D5426
>
> Modified:
>   head/sys/netpfil/pf/pf_ioctl.c
>
> Modified: head/sys/netpfil/pf/pf_ioctl.c
> ==============================================================================
> --- head/sys/netpfil/pf/pf_ioctl.c      Thu Feb 25 07:03:10 2016        (r296024)
> +++ head/sys/netpfil/pf/pf_ioctl.c      Thu Feb 25 07:33:59 2016        (r296025)
> @@ -2718,13 +2718,14 @@ DIOCCHANGEADDR_error:
>         case DIOCRSETADDRS: {
>                 struct pfioc_table *io = (struct pfioc_table *)addr;
>                 struct pfr_addr *pfras;
> -               size_t totlen;
> +               size_t totlen, count;
>
>                 if (io->pfrio_esize != sizeof(struct pfr_addr)) {
>                         error = ENODEV;
>                         break;
>                 }
> -               totlen = io->pfrio_size * sizeof(struct pfr_addr);
> +               count = max(io->pfrio_size, io->pfrio_size2);
> +               totlen = count * sizeof(struct pfr_addr);
>                 pfras = malloc(totlen, M_TEMP, M_WAITOK);
>                 error = copyin(io->pfrio_buffer, pfras, totlen);
>                 if (error) {
>