From owner-dev-commits-src-branches@freebsd.org Wed Feb 3 14:20:10 2021
Return-Path: <owner-dev-commits-src-branches@freebsd.org>
Delivered-To: dev-commits-src-branches@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 <kp@FreeBSD.org>
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-branches@freebsd.org
X-Mailman-Version: 2.1.34
Precedence: list
List-Id: Commits to the stable branches of the FreeBSD src repository
<dev-commits-src-branches.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/dev-commits-src-branches>,
<mailto:dev-commits-src-branches-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/dev-commits-src-branches/>
List-Post: <mailto:dev-commits-src-branches@freebsd.org>
List-Help: <mailto:dev-commits-src-branches-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/dev-commits-src-branches>,
<mailto:dev-commits-src-branches-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 03 Feb 2021 14:20:11 -0000
The branch stable/12 has been updated by kp:
URL: https://cgit.FreeBSD.org/src/commit/?id=71abbe15d01f73a4e55a732f21180d339eab631d
commit 71abbe15d01f73a4e55a732f21180d339eab631d
Author: Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-01-26 07:56:51 +0000
Commit: Kristof Provost <kp@FreeBSD.org>
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);