From ed6b72629f0e2e85c06c46358172f3f178261f09 Mon Sep 17 00:00:00 2001 From: kn Date: Wed, 16 Dec 2020 18:00:45 +0000 Subject: Reject rules with invalid port ranges Ranges where the left boundary is bigger than the right one are always bogus as they work like `port any' (`port 34<>12' means "all ports") or in way that inverts the rule's action (`pass ... port 34:12' means "pass no port at all"). Add checks for all ranges and invalidate those that yield no or all ports. For this to work on redirections, make pfctl(8) pass the range's type, otherwise boundary including ranges are not detected as such; that is to say, `struct pf_pool's `port_op' member was unused in the kernel so far. `rdr-to' rules with invalid ranges could panic the kernel when hit. Reported-by: syzbot+9c309db201f06e39a8ba@syzkaller.appspotmail.com OK sashan --- sys/net/pf_ioctl.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'sys/net') diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c index 27315239807..afa40bb677b 100644 --- a/sys/net/pf_ioctl.c +++ b/sys/net/pf_ioctl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_ioctl.c,v 1.360 2020/10/22 12:25:20 sashan Exp $ */ +/* $OpenBSD: pf_ioctl.c,v 1.361 2020/12/16 18:00:44 kn Exp $ */ /* * Copyright (c) 2001 Daniel Hartmeier @@ -107,6 +107,7 @@ int pf_kif_setup(char *, struct pfi_kif **); void pf_addr_copyout(struct pf_addr_wrap *); void pf_trans_set_commit(void); void pf_pool_copyin(struct pf_pool *, struct pf_pool *); +int pf_validate_range(u_int8_t, u_int16_t[2]); int pf_rule_copyin(struct pf_rule *, struct pf_rule *, struct pf_ruleset *); u_int16_t pf_qname2qid(char *, int); @@ -2943,6 +2944,19 @@ pf_pool_copyin(struct pf_pool *from, struct pf_pool *to) to->addr.p.tbl = NULL; } +int +pf_validate_range(u_int8_t op, u_int16_t port[2]) +{ + u_int16_t a = ntohs(port[0]); + u_int16_t b = ntohs(port[1]); + + if ((op == PF_OP_RRG && a > b) || /* 34:12, i.e. none */ + (op == PF_OP_IRG && a >= b) || /* 34><12, i.e. none */ + (op == PF_OP_XRG && a > b)) /* 34<>22, i.e. all */ + return 1; + return 0; +} + int pf_rule_copyin(struct pf_rule *from, struct pf_rule *to, struct pf_ruleset *ruleset) @@ -2954,6 +2968,11 @@ pf_rule_copyin(struct pf_rule *from, struct pf_rule *to, to->dst = from->dst; to->dst.addr.p.tbl = NULL; + if (pf_validate_range(to->src.port_op, to->src.port)) + return (EINVAL); + if (pf_validate_range(to->dst.port_op, to->dst.port)) + return (EINVAL); + /* XXX union skip[] */ strlcpy(to->label, from->label, sizeof(to->label)); @@ -2971,6 +2990,9 @@ pf_rule_copyin(struct pf_rule *from, struct pf_rule *to, pf_pool_copyin(&from->rdr, &to->rdr); pf_pool_copyin(&from->route, &to->route); + if (pf_validate_range(to->rdr.port_op, to->rdr.proxy_port)) + return (EINVAL); + if (pf_kif_setup(to->ifname, &to->kif)) return (EINVAL); if (pf_kif_setup(to->rcv_ifname, &to->rcv_kif)) -- cgit v1.2.3