diff options
| author | Kristof Provost <kp@FreeBSD.org> | 2021-01-26 07:56:51 +0000 |
|---|---|---|
| committer | Kristof Provost <kp@FreeBSD.org> | 2021-01-27 15:42:14 +0000 |
| commit | 7a808c5ee3296fdb72d8e8bc6c7ad6f316a520ab (patch) | |
| tree | dc62817d1d9cf74559f563ebdcee8544fb289e47 | |
| parent | 5c325977b1138828367f39a3f2034af24c3654f0 (diff) | |
| download | src-7a808c5ee3296fdb72d8e8bc6c7ad6f316a520ab.tar.gz src-7a808c5ee3296fdb72d8e8bc6c7ad6f316a520ab.zip | |
pf: Improve pf_rule input validation
Move the validation checks to pf_rule_to_krule() to reduce duplication.
This also makes the checks consistent across different ioctls.
Reported-by: syzbot+e9632d7ad17398f0bd8f@syzkaller.appspotmail.com
Reviewed by: tuexen@, donner@
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D28362
| -rw-r--r-- | sys/netpfil/pf/pf_ioctl.c | 72 |
1 files changed, 41 insertions, 31 deletions
diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 60c38a980d1e..644a091808cd 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -1557,10 +1557,39 @@ pf_krule_to_rule(const struct pf_krule *krule, struct pf_rule *rule) rule->u_src_nodes = counter_u64_fetch(krule->src_nodes); } -static void +static int pf_rule_to_krule(const struct pf_rule *rule, struct pf_krule *krule) { +#ifndef INET + if (rule->af == AF_INET) { + return (EAFNOSUPPORT); + } +#endif /* INET */ +#ifndef INET6 + if (rule->af == AF_INET6) { + return (EAFNOSUPPORT); + } +#endif /* INET6 */ + + if (rule->src.addr.type != PF_ADDR_ADDRMASK && + rule->src.addr.type != PF_ADDR_DYNIFTL && + rule->src.addr.type != PF_ADDR_TABLE) { + return (EINVAL); + } + if (rule->src.addr.p.dyn != NULL) { + return (EINVAL); + } + + if (rule->dst.addr.type != PF_ADDR_ADDRMASK && + rule->dst.addr.type != PF_ADDR_DYNIFTL && + rule->dst.addr.type != PF_ADDR_TABLE) { + return (EINVAL); + } + if (rule->dst.addr.p.dyn != NULL) { + return (EINVAL); + } + bzero(krule, sizeof(*krule)); bcopy(&rule->src, &krule->src, sizeof(rule->src)); @@ -1641,6 +1670,8 @@ pf_rule_to_krule(const struct pf_rule *rule, struct pf_krule *krule) krule->set_prio[1] = rule->set_prio[1]; bcopy(&rule->divert, &krule->divert, sizeof(krule->divert)); + + return (0); } static int @@ -1815,26 +1846,13 @@ pfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *td error = EINVAL; break; } - if (pr->rule.src.addr.p.dyn != NULL || - pr->rule.dst.addr.p.dyn != NULL) { - error = EINVAL; - break; - } -#ifndef INET - if (pr->rule.af == AF_INET) { - error = EAFNOSUPPORT; - break; - } -#endif /* INET */ -#ifndef INET6 - if (pr->rule.af == AF_INET6) { - error = EAFNOSUPPORT; - break; - } -#endif /* INET6 */ rule = malloc(sizeof(*rule), M_PFRULE, M_WAITOK); - pf_rule_to_krule(&pr->rule, rule); + error = pf_rule_to_krule(&pr->rule, rule); + if (error != 0) { + free(rule, M_PFRULE); + break; + } if (rule->ifname[0]) kif = pf_kkif_create(M_WAITOK); @@ -2090,20 +2108,12 @@ DIOCADDRULE_error: } if (pcr->action != PF_CHANGE_REMOVE) { -#ifndef INET - if (pcr->rule.af == AF_INET) { - error = EAFNOSUPPORT; - break; - } -#endif /* INET */ -#ifndef INET6 - if (pcr->rule.af == AF_INET6) { - error = EAFNOSUPPORT; + newrule = malloc(sizeof(*newrule), M_PFRULE, M_WAITOK); + error = pf_rule_to_krule(&pcr->rule, newrule); + if (error != 0) { + free(newrule, M_PFRULE); break; } -#endif /* INET6 */ - newrule = malloc(sizeof(*newrule), M_PFRULE, M_WAITOK); - pf_rule_to_krule(&pcr->rule, newrule); if (newrule->ifname[0]) kif = pf_kkif_create(M_WAITOK); |