Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 6 Apr 2018 15:01:46 +0000 (UTC)
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r332101 - head/sys/netpfil/pf
Message-ID:  <201804061501.w36F1kaq086080@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kp
Date: Fri Apr  6 15:01:45 2018
New Revision: 332101
URL: https://svnweb.freebsd.org/changeset/base/332101

Log:
  pf: Improve ioctl validation for DIOCRADDTABLES and DIOCRDELTABLES
  
  The DIOCRADDTABLES and DIOCRDELTABLES ioctls can process a number of
  tables at a time, and as such try to allocate <number of tables> *
  sizeof(struct pfr_table). This multiplication can overflow. Thanks to
  mallocarray() this is not exploitable, but an overflow does panic the
  system.
  
  Arbitrarily limit this to 65535 tables. pfctl only ever processes one
  table at a time, so it presents no issues there.
  
  MFC after:	1 week

Modified:
  head/sys/netpfil/pf/pf_ioctl.c

Modified: head/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- head/sys/netpfil/pf/pf_ioctl.c	Fri Apr  6 13:00:45 2018	(r332100)
+++ head/sys/netpfil/pf/pf_ioctl.c	Fri Apr  6 15:01:45 2018	(r332101)
@@ -89,6 +89,8 @@ __FBSDID("$FreeBSD$");
 #include <net/altq/altq.h>
 #endif
 
+#define PF_TABLES_MAX_REQUEST   65535 /* Maximum tables per request. */
+
 static struct pf_pool	*pf_get_pool(char *, u_int32_t, u_int8_t, u_int32_t,
 			    u_int8_t, u_int8_t, u_int8_t);
 
@@ -2530,13 +2532,15 @@ DIOCCHANGEADDR_error:
 			error = ENODEV;
 			break;
 		}
-		totlen = io->pfrio_size * sizeof(struct pfr_table);
-		pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
-		    M_TEMP, M_WAITOK);
-		if (! pfrts) {
+
+		if (io->pfrio_size < 0 || io->pfrio_size > PF_TABLES_MAX_REQUEST) {
 			error = ENOMEM;
 			break;
 		}
+
+		totlen = io->pfrio_size * sizeof(struct pfr_table);
+		pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+		    M_TEMP, M_WAITOK);
 		error = copyin(io->pfrio_buffer, pfrts, totlen);
 		if (error) {
 			free(pfrts, M_TEMP);
@@ -2559,13 +2563,15 @@ DIOCCHANGEADDR_error:
 			error = ENODEV;
 			break;
 		}
-		totlen = io->pfrio_size * sizeof(struct pfr_table);
-		pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
-		    M_TEMP, M_WAITOK);
-		if (! pfrts) {
+
+		if (io->pfrio_size < 0 || io->pfrio_size > PF_TABLES_MAX_REQUEST) {
 			error = ENOMEM;
 			break;
 		}
+
+		totlen = io->pfrio_size * sizeof(struct pfr_table);
+		pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+		    M_TEMP, M_WAITOK);
 		error = copyin(io->pfrio_buffer, pfrts, totlen);
 		if (error) {
 			free(pfrts, M_TEMP);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201804061501.w36F1kaq086080>