Download raw body.
sys/net/pf_ioctl.c: out of bounds read in pf_rule_copyin
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));
sys/net/pf_ioctl.c: out of bounds read in pf_rule_copyin