From owner-dev-commits-src-all@freebsd.org Wed Feb 3 14:20:10 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B58D25380C2; Wed, 3 Feb 2021 14:20:10 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DW3jk2fgNz4f1r; Wed, 3 Feb 2021 14:20:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id CB4F0222E0; Wed, 3 Feb 2021 14:20:08 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 113EK8O9078374; Wed, 3 Feb 2021 14:20:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 113EK8Ig078372; Wed, 3 Feb 2021 14:20:08 GMT (envelope-from git) Date: Wed, 3 Feb 2021 14:20:08 GMT Message-Id: <202102031420.113EK8Ig078372@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kristof Provost Subject: git: 71abbe15d01f - stable/12 - pf: Improve pf_rule input validation MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/stable/12 X-Git-Reftype: branch X-Git-Commit: 71abbe15d01f73a4e55a732f21180d339eab631d Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Feb 2021 14:20:13 -0000 The branch stable/12 has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=71abbe15d01f73a4e55a732f21180d339eab631d commit 71abbe15d01f73a4e55a732f21180d339eab631d Author: Kristof Provost AuthorDate: 2021-01-26 07:56:51 +0000 Commit: Kristof Provost CommitDate: 2021-02-03 09:09:14 +0000 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 (cherry picked from commit 7a808c5ee3296fdb72d8e8bc6c7ad6f316a520ab) --- sys/netpfil/pf/pf_ioctl.c | 72 +++++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 48f68fa249e2..bbb9cfe39586 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -1558,10 +1558,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)); @@ -1642,6 +1671,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 @@ -1816,26 +1847,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); @@ -2092,20 +2110,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);