Skip to content

Commit

Permalink
check pf rule "set prio" values consistently.
Browse files Browse the repository at this point in the history
consistently means we do the check in pf_rule_copyin() so both
DIOCADDRULE and DIOCCHANGERULE have the prio values checked. this in
turn prevents invalid prio values getting set on a rule via
DIOCCHANGERULE.

this was caught by a kassert in the ifq priq code firing.

Reported-by: syzbot+a8f8e24a44b441e71d93@syzkaller.appspotmail.com
ok sashan@
  • Loading branch information
dlg committed Feb 16, 2022
1 parent 56d79bc commit 2960c8a
Showing 1 changed file with 6 additions and 10 deletions.
16 changes: 6 additions & 10 deletions sys/net/pf_ioctl.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: pf_ioctl.c,v 1.372 2022/02/09 11:42:58 sashan Exp $ */
/* $OpenBSD: pf_ioctl.c,v 1.373 2022/02/16 04:25:34 dlg Exp $ */

/*
* Copyright (c) 2001 Daniel Hartmeier
Expand Down Expand Up @@ -1370,15 +1370,6 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags, struct proc *p)
break;
}

if (rule->scrub_flags & PFSTATE_SETPRIO &&
(rule->set_prio[0] > IFQ_MAXPRIO ||
rule->set_prio[1] > IFQ_MAXPRIO)) {
error = EINVAL;
pf_rule_free(rule);
rule = NULL;
break;
}

NET_LOCK();
PF_LOCK();
pr->anchor[sizeof(pr->anchor) - 1] = '\0';
Expand Down Expand Up @@ -3071,6 +3062,11 @@ pf_rule_copyin(struct pf_rule *from, struct pf_rule *to)
{
int i;

if (from->scrub_flags & PFSTATE_SETPRIO &&
(from->set_prio[0] > IFQ_MAXPRIO ||
from->set_prio[1] > IFQ_MAXPRIO))
return (EINVAL);

to->src = from->src;
to->src.addr.p.tbl = NULL;
to->dst = from->dst;
Expand Down

0 comments on commit 2960c8a

Please sign in to comment.