Index | Thread | Search

From:
Johann Höpfner <hoepf@cit.tum.de>
Subject:
sys/net/pf_ioctl.c: out of bounds read in pf_rule_copyin
To:
tech@openbsd.org
Cc:
hoepf@cit.tum.de
Date:
Mon, 6 Jul 2026 12:05:31 +0200

Download raw body.

Thread
Hello tech@

In testing an experimental kernel-address-sanitizer in syzkaller 
the following issue was discovered. (and independently found by the
implementation running in syzbot this morning:
https://syzkaller.appspot.com/bug?extid=f028f6e8a734a5e8de11)

pf_rule_copyin executes strlcpy on non-null-terminated fields from 
userspace leading to a read beyond the allocation pointed to by "from", 
only if userspace sends a struct pf_rule without zero-terminators, e.g.
as follows. 

#include <fcntl.h>
#include <string.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <net/pfvar.h>
int main(){
        int fd = open("/dev/pf", O_RDWR);
        struct pfioc_rule pr;
        memset(&pr, 'A', sizeof(struct pfioc_rule));
        ioctl(fd, DIOCADDRULE, &pr);
}

Due to "from" being allocated in a 4096 byte bucket this is very unlikely
to cause a crash without kasan.

I suggest the following fix terminating the fields before strlcpy. A 
strscpy-like approach might be cleaner as we discard the return value.

Index: sys/net/pf_ioctl.c
===================================================================
--- sys/net/pf_ioctl.c
+++ sys/net/pf_ioctl.c
@@ -4040,6 +4040,15 @@
 
 	/* XXX union skip[] */
 
+	from->label[sizeof(from->label) - 1] = '\0';
+	from->ifname[sizeof(from->ifname) - 1] = '\0';
+	from->rcv_ifname[sizeof(from->rcv_ifname) - 1] = '\0';
+	from->qname[sizeof(from->qname) - 1] = '\0';
+	from->pqname[sizeof(from->pqname) - 1] = '\0';
+	from->tagname[sizeof(from->tagname) - 1] = '\0';
+	from->match_tagname[sizeof(from->match_tagname) - 1] = '\0';
+	from->overload_tblname[sizeof(from->overload_tblname) - 1] = '\0';
+
 	strlcpy(to->label, from->label, sizeof(to->label));
 	strlcpy(to->ifname, from->ifname, sizeof(to->ifname));
 	strlcpy(to->rcv_ifname, from->rcv_ifname, sizeof(to->rcv_ifname));