git: eda1756d0454 - main - ipfilter: Verify frentry on entry into kernel
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 26 Nov 2025 15:18:39 UTC
The branch main has been updated by cy:
URL: https://cgit.FreeBSD.org/src/commit/?id=eda1756d0454f9383940dc825cf571ff67e0c013
commit eda1756d0454f9383940dc825cf571ff67e0c013
Author: Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2025-10-29 17:23:23 +0000
Commit: Cy Schubert <cy@FreeBSD.org>
CommitDate: 2025-11-26 15:16:46 +0000
ipfilter: Verify frentry on entry into kernel
The frentry struct is built by ipf(8), specifically ipf_y.y when parsing
the ipfilter configuration file (typically ipf.conf). frentry contains
a variable length string field at the end of the struct. This data field,
called fr_names, may contain various text strings such as NIC names,
destination list (dstlist) names, and filter rule comments. The length
field specifies the length of fr_names within the frentry structure and
fr_size specifies the size of the frentry structure itself.
The upper bound limit to the length of strings field is controlled by the
fr_max_namelen sysctl/kenv or the max_namelen ipfilter tuneable.
The initial concepts were discussed with emaste and jrm.
Reported by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Reviewed by: markj
MFC after: 1 week
Differential revision: https://reviews.freebsd.org/D53843
---
sbin/ipf/libipf/interror.c | 5 +++
sys/netpfil/ipfilter/netinet/fil.c | 61 +++++++++++++++++++++++++++++++--
sys/netpfil/ipfilter/netinet/ip_fil.h | 1 +
sys/netpfil/ipfilter/netinet/mlfk_ipl.c | 1 +
4 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/sbin/ipf/libipf/interror.c b/sbin/ipf/libipf/interror.c
index a8dc3be2d5d1..4d48825bb3ad 100644
--- a/sbin/ipf/libipf/interror.c
+++ b/sbin/ipf/libipf/interror.c
@@ -177,6 +177,11 @@ static ipf_error_entry_t ipf_errors[] = {
{ 149, "object size validation failed for kernel copyout" },
{ 150, "error copying data out for kernel copyout" },
{ 151, "version mismatch for kernel copyout" },
+ { 152, "fr_names offset is wrapped negative" },
+ { 153, "fr_names larger than fr_namelen" },
+ { 154, "frentry larger than fr_size" },
+ { 155, "frentry and fr_namelen mismatch fr_size" },
+ { 156, "fr_namelen too large" },
/* -------------------------------------------------------------------------- */
{ 10001, "could not find token for auth iterator" },
{ 10002, "write permissions require to add/remove auth rule" },
diff --git a/sys/netpfil/ipfilter/netinet/fil.c b/sys/netpfil/ipfilter/netinet/fil.c
index d487cdde20d8..0de5378322df 100644
--- a/sys/netpfil/ipfilter/netinet/fil.c
+++ b/sys/netpfil/ipfilter/netinet/fil.c
@@ -363,6 +363,10 @@ static ipftuneable_t ipf_main_tuneables[] = {
"ip_timeout", 1, 0x7fffffff,
stsizeof(ipf_main_softc_t, ipf_iptimeout),
0, NULL, ipf_settimeout },
+ { { (void *)offsetof(ipf_main_softc_t, ipf_max_namelen) },
+ "max_namelen", 0, 0x7fffffff,
+ stsizeof(ipf_main_softc_t, ipf_max_namelen),
+ 0, NULL, NULL },
#if defined(INSTANCES) && defined(_KERNEL)
{ { (void *)offsetof(ipf_main_softc_t, ipf_get_loopback) },
"intercept_loopback", 0, 1,
@@ -4399,7 +4403,8 @@ int
frrequest(ipf_main_softc_t *softc, int unit, ioctlcmd_t req, caddr_t data,
int set, int makecopy)
{
- int error = 0, in, family, need_free = 0;
+ int error = 0, in, family, need_free = 0, interr, i;
+ int interr_tbl[3] = { 152, 156, 153};
enum { OP_ADD, /* add rule */
OP_REM, /* remove rule */
OP_ZERO /* zero statistics and counters */ }
@@ -4408,7 +4413,9 @@ frrequest(ipf_main_softc_t *softc, int unit, ioctlcmd_t req, caddr_t data,
void *ptr, *uptr;
u_int *p, *pp;
frgroup_t *fg;
- char *group;
+ char *group, *name;
+ size_t v_fr_size, v_element_size;
+ int v_rem_namelen, v_fr_toend;
ptr = NULL;
fg = NULL;
@@ -4423,6 +4430,17 @@ frrequest(ipf_main_softc_t *softc, int unit, ioctlcmd_t req, caddr_t data,
IPFERROR(6);
return (EINVAL);
}
+ if (fp->fr_size < sizeof(frd)) {
+ return (EINVAL);
+ }
+ if (sizeof(frd) + fp->fr_namelen != fp->fr_size ) {
+ IPFERROR(155);
+ return (EINVAL);
+ }
+ if (fp->fr_namelen < 0 || fp->fr_namelen > softc->ipf_max_namelen) {
+ IPFERROR(156);
+ return (EINVAL);
+ }
KMALLOCS(f, frentry_t *, fp->fr_size);
if (f == NULL) {
IPFERROR(131);
@@ -4449,6 +4467,44 @@ frrequest(ipf_main_softc_t *softc, int unit, ioctlcmd_t req, caddr_t data,
fp->fr_ptr = NULL;
fp->fr_ref = 0;
fp->fr_flags |= FR_COPIED;
+
+ for (i = 0; i <= 3; i++) {
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_ifnames[i])) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_comment)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_group)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_grhead)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_tif.fd_name)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_rif.fd_name)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
+ if ((interr = ipf_check_names_string(fp->fr_names, fp->fr_namelen, fp->fr_dif.fd_name)) != 0) {
+ IPFERROR(interr_tbl[interr-1]);
+ error = EINVAL;
+ goto donenolock;
+ }
} else {
fp = (frentry_t *)data;
if ((fp->fr_type & FR_T_BUILTIN) == 0) {
@@ -9040,6 +9096,7 @@ ipf_main_soft_create(void *arg)
#endif
softc->ipf_minttl = 4;
softc->ipf_icmpminfragmtu = 68;
+ softc->ipf_max_namelen = 128;
softc->ipf_flags = IPF_LOGGING;
#ifdef LARGE_NAT
diff --git a/sys/netpfil/ipfilter/netinet/ip_fil.h b/sys/netpfil/ipfilter/netinet/ip_fil.h
index ad6128d9a8e2..7b070f0d6867 100644
--- a/sys/netpfil/ipfilter/netinet/ip_fil.h
+++ b/sys/netpfil/ipfilter/netinet/ip_fil.h
@@ -1529,6 +1529,7 @@ typedef struct ipf_main_softc_s {
int ipf_pass;
int ipf_minttl;
int ipf_icmpminfragmtu;
+ int ipf_max_namelen;
int ipf_interror; /* Should be in a struct that is per */
/* thread or process. Does not belong */
/* here but there's a lot more work */
diff --git a/sys/netpfil/ipfilter/netinet/mlfk_ipl.c b/sys/netpfil/ipfilter/netinet/mlfk_ipl.c
index 1c3051fb6615..d558b2d24b2c 100644
--- a/sys/netpfil/ipfilter/netinet/mlfk_ipl.c
+++ b/sys/netpfil/ipfilter/netinet/mlfk_ipl.c
@@ -135,6 +135,7 @@ SYSCTL_IPF(_net_inet_ipf, OID_AUTO, fr_running, CTLFLAG_RD,
SYSCTL_IPF(_net_inet_ipf, OID_AUTO, fr_chksrc, CTLFLAG_RW, &VNET_NAME(ipfmain.ipf_chksrc), 0, "");
SYSCTL_IPF(_net_inet_ipf, OID_AUTO, fr_minttl, CTLFLAG_RW, &VNET_NAME(ipfmain.ipf_minttl), 0, "");
SYSCTL_IPF(_net_inet_ipf, OID_AUTO, large_nat, CTLFLAG_RDTUN | CTLFLAG_NOFETCH, &VNET_NAME(ipfmain.ipf_large_nat), 0, "large_nat");
+SYSCTL_IPF(_net_inet_ipf, OID_AUTO, fr_max_namelen, CTLFLAG_RWTUN, &VNET_NAME(ipfmain.ipf_max_namelen), 0, "max_namelen");
#define CDEV_MAJOR 79
#include <sys/poll.h>