The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=2be46b52f5db0630550ec60ad8f92a7e7d7d78ab
commit 2be46b52f5db0630550ec60ad8f92a7e7d7d78ab Author: Kristof Provost <[email protected]> AuthorDate: 2025-08-27 19:32:33 +0000 Commit: Kristof Provost <[email protected]> CommitDate: 2025-09-25 12:41:09 +0000 pfctl: fix once rules parse.y revision 1.682 from 16.07.2018 errornously allowed `match once' and `anchor "a" once'. Fix both by checking for PF_DROP not PF_MATCH and creating anchors in the parser already such that they can be used to distinguish anchor rules in the same check as well. Found and fixed by Petr Hoffmann <petr.hoffmann at oracle dot com>, thanks! While here, remove an unneeded cast and make pfctl_add_rule() void as it always returned 0. OK sashan Obtained from: OpenBSD, kn <[email protected]>, 6da84b37b3 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sbin/pfctl/parse.y | 69 +++++++++++++++++++++++++++++++---------------- sbin/pfctl/pfctl.c | 30 ++------------------- sbin/pfctl/pfctl_parser.h | 2 +- 3 files changed, 49 insertions(+), 52 deletions(-) diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y index c75632c740b3..babe6b99013e 100644 --- a/sbin/pfctl/parse.y +++ b/sbin/pfctl/parse.y @@ -372,8 +372,8 @@ int validate_range(uint8_t, uint16_t, uint16_t); int disallow_table(struct node_host *, const char *); int disallow_urpf_failed(struct node_host *, const char *); int disallow_alias(struct node_host *, const char *); -int rule_consistent(struct pfctl_rule *, int); -int filter_consistent(struct pfctl_rule *, int); +int rule_consistent(struct pfctl_rule *); +int filter_consistent(struct pfctl_rule *); int nat_consistent(struct pfctl_rule *); int rdr_consistent(struct pfctl_rule *); int process_tabledef(char *, struct table_opts *, int); @@ -403,7 +403,7 @@ void expand_rule(struct pfctl_rule *, bool, struct node_if *, struct node_proto *, struct node_os *, struct node_host *, struct node_port *, struct node_host *, struct node_port *, struct node_uid *, struct node_gid *, struct node_if *, - struct node_icmp *, const char *); + struct node_icmp *); int expand_altq(struct pf_altq *, struct node_if *, struct node_queue *, struct node_queue_bw bwspec, struct node_queue_opt *); @@ -994,6 +994,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto { struct pfctl_rule r; struct node_proto *proto; + char *p; if (check_rulestate(PFCTL_STATE_FILTER)) { if ($2) @@ -1039,6 +1040,30 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto "rules must specify a name"); YYERROR; } + /* + * Don't make non-brace anchors part of the main anchor pool. + */ + if ((r.anchor = calloc(1, sizeof(*r.anchor))) == NULL) { + err(1, "anchorrule: calloc"); + } + pf_init_ruleset(&r.anchor->ruleset); + r.anchor->ruleset.anchor = r.anchor; + if (strlcpy(r.anchor->path, $2, + sizeof(r.anchor->path)) >= sizeof(r.anchor->path)) { + errx(1, "anchorrule: strlcpy"); + } + if ((p = strrchr($2, '/')) != NULL) { + if (strlen(p) == 1) { + yyerror("anchorrule: bad anchor name %s", + $2); + YYERROR; + } + } else + p = $2; + if (strlcpy(r.anchor->name, p, + sizeof(r.anchor->name)) >= sizeof(r.anchor->name)) { + errx(1, "anchorrule: strlcpy"); + } } r.direction = $3; r.quick = $4.quick; @@ -1075,8 +1100,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto expand_rule(&r, false, $5, NULL, NULL, NULL, $7, $8.src_os, $8.src.host, $8.src.port, $8.dst.host, - $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec, - pf->astack[pf->asd + 1] ? pf->alast->name : $2); + $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec); free($2); pf->astack[pf->asd + 1] = NULL; } @@ -1099,7 +1123,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto expand_rule(&r, false, $3, NULL, NULL, NULL, $5, $6.src_os, $6.src.host, $6.src.port, $6.dst.host, - $6.dst.port, 0, 0, 0, 0, $2); + $6.dst.port, 0, 0, 0, 0); free($2); } | RDRANCHOR string interface af proto fromto rtable { @@ -1142,7 +1166,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto expand_rule(&r, false, $3, NULL, NULL, NULL, $5, $6.src_os, $6.src.host, $6.src.port, $6.dst.host, - $6.dst.port, 0, 0, 0, 0, $2); + $6.dst.port, 0, 0, 0, 0); free($2); } | BINATANCHOR string interface af proto fromto rtable { @@ -1178,7 +1202,7 @@ anchorrule : ANCHOR anchorname dir quick interface af proto fromto decide_address_family($6.src.host, &r.af); decide_address_family($6.dst.host, &r.af); - pfctl_append_rule(pf, &r, $2); + pfctl_append_rule(pf, &r); free($2); } ; @@ -1465,7 +1489,7 @@ scrubrule : scrubaction dir logquick interface af proto fromto scrub_opts expand_rule(&r, false, $4, NULL, NULL, NULL, $6, $7.src_os, $7.src.host, $7.src.port, $7.dst.host, - $7.dst.port, NULL, NULL, NULL, NULL, ""); + $7.dst.port, NULL, NULL, NULL, NULL); } ; @@ -1630,7 +1654,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af antispoof_opts { if (h != NULL) expand_rule(&r, false, j, NULL, NULL, NULL, NULL, NULL, h, NULL, NULL, - NULL, NULL, NULL, NULL, NULL, ""); + NULL, NULL, NULL, NULL, NULL); if ((i->ifa_flags & IFF_LOOPBACK) == 0) { bzero(&r, sizeof(r)); @@ -1653,7 +1677,7 @@ antispoof : ANTISPOOF logquick antispoof_ifspc af antispoof_opts { expand_rule(&r, false, NULL, NULL, NULL, NULL, NULL, NULL, h, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, ""); + NULL, NULL, NULL, NULL); } else free(hh); } @@ -2736,7 +2760,7 @@ pfrule : action dir logquick interface route af proto fromto expand_rule(&r, false, $4, $9.nat, $9.rdr, $5.redirspec, $7, $8.src_os, $8.src.host, $8.src.port, $8.dst.host, - $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec, ""); + $8.dst.port, $9.uid, $9.gid, $9.rcv, $9.icmpspec); } ; @@ -4871,7 +4895,7 @@ natrule : nataction interface af proto fromto tag tagged rtable expand_rule(&r, false, $2, NULL, $9, NULL, $4, $5.src_os, $5.src.host, $5.src.port, $5.dst.host, - $5.dst.port, 0, 0, 0, 0, ""); + $5.dst.port, 0, 0, 0, 0); } ; @@ -5050,7 +5074,7 @@ binatrule : no BINAT natpasslog interface af proto FROM ipspec toipspec tag free($13); } - pfctl_append_rule(pf, &binat, ""); + pfctl_append_rule(pf, &binat); } ; @@ -5277,7 +5301,7 @@ disallow_alias(struct node_host *h, const char *fmt) } int -rule_consistent(struct pfctl_rule *r, int anchor_call) +rule_consistent(struct pfctl_rule *r) { int problems = 0; @@ -5287,7 +5311,7 @@ rule_consistent(struct pfctl_rule *r, int anchor_call) case PF_DROP: case PF_SCRUB: case PF_NOSCRUB: - problems = filter_consistent(r, anchor_call); + problems = filter_consistent(r); break; case PF_NAT: case PF_NONAT: @@ -5306,7 +5330,7 @@ rule_consistent(struct pfctl_rule *r, int anchor_call) } int -filter_consistent(struct pfctl_rule *r, int anchor_call) +filter_consistent(struct pfctl_rule *r) { int problems = 0; @@ -6346,7 +6370,7 @@ expand_rule(struct pfctl_rule *r, bool keeprule, struct node_os *src_oses, struct node_host *src_hosts, struct node_port *src_ports, struct node_host *dst_hosts, struct node_port *dst_ports, struct node_uid *uids, struct node_gid *gids, - struct node_if *rcv, struct node_icmp *icmp_types, const char *anchor_call) + struct node_if *rcv, struct node_icmp *icmp_types) { sa_family_t af = r->af; int added = 0, error = 0; @@ -6513,11 +6537,11 @@ expand_rule(struct pfctl_rule *r, bool keeprule, error += check_binat_redirspec(src_host, r, af); } - if (rule_consistent(r, anchor_call[0]) < 0 || error) + if (rule_consistent(r) < 0 || error) yyerror("skipping rule due to errors"); else { r->nr = pf->astack[pf->asd]->match++; - pfctl_append_rule(pf, r, anchor_call); + pfctl_append_rule(pf, r); added++; } @@ -6533,8 +6557,7 @@ expand_rule(struct pfctl_rule *r, bool keeprule, expand_rule(&rdr_rule, true, interface, NULL, rdr_redirspec, NULL, proto, src_os, dst_host, dst_port, - rdr_dst_host, src_port, uid, gid, rcv, icmp_type, - ""); + rdr_dst_host, src_port, uid, gid, rcv, icmp_type); } if (osrch && src_host->addr.type == PF_ADDR_DYNIFTL) { @@ -7743,7 +7766,7 @@ int filteropts_to_rule(struct pfctl_rule *r, struct filter_opts *opts) { if (opts->marker & FOM_ONCE) { - if (r->action != PF_PASS && r->action != PF_MATCH) { + if ((r->action != PF_PASS && r->action != PF_DROP) || r->anchor) { yyerror("'once' only applies to pass/block rules"); return (1); } diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c index 7e1195fdf2f0..b8f4305a3e38 100644 --- a/sbin/pfctl/pfctl.c +++ b/sbin/pfctl/pfctl.c @@ -1861,14 +1861,12 @@ pfctl_init_rule(struct pfctl_rule *r) TAILQ_INIT(&(r->route.list)); } -int -pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r, - const char *anchor_call) +void +pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r) { u_int8_t rs_num; struct pfctl_rule *rule; struct pfctl_ruleset *rs; - char *p; rs_num = pf_get_ruleset_number(r->action); if (rs_num == PF_RULESET_MAX) @@ -1876,29 +1874,6 @@ pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r, rs = &pf->anchor->ruleset; - if (anchor_call[0] && r->anchor == NULL) { - /* - * Don't make non-brace anchors part of the main anchor pool. - */ - if ((r->anchor = calloc(1, sizeof(*r->anchor))) == NULL) - err(1, "pfctl_append_rule: calloc"); - - pf_init_ruleset(&r->anchor->ruleset); - r->anchor->ruleset.anchor = r->anchor; - if (strlcpy(r->anchor->path, anchor_call, - sizeof(rule->anchor->path)) >= sizeof(rule->anchor->path)) - errx(1, "pfctl_append_rule: strlcpy"); - if ((p = strrchr(anchor_call, '/')) != NULL) { - if (!strlen(p)) - err(1, "pfctl_append_rule: bad anchor name %s", - anchor_call); - } else - p = (char *)anchor_call; - if (strlcpy(r->anchor->name, p, - sizeof(rule->anchor->name)) >= sizeof(rule->anchor->name)) - errx(1, "pfctl_append_rule: strlcpy"); - } - if ((rule = calloc(1, sizeof(*rule))) == NULL) err(1, "calloc"); bcopy(r, rule, sizeof(*rule)); @@ -1910,7 +1885,6 @@ pfctl_append_rule(struct pfctl *pf, struct pfctl_rule *r, pfctl_move_pool(&r->route, &rule->route); TAILQ_INSERT_TAIL(rs->rules[rs_num].active.ptr, rule, entries); - return (0); } int diff --git a/sbin/pfctl/pfctl_parser.h b/sbin/pfctl/pfctl_parser.h index e96ff0195e03..44ddfb45fbe1 100644 --- a/sbin/pfctl/pfctl_parser.h +++ b/sbin/pfctl/pfctl_parser.h @@ -288,7 +288,7 @@ int pfctl_rules(int, char *, int, int, char *, struct pfr_buffer *); int pfctl_optimize_ruleset(struct pfctl *, struct pfctl_ruleset *); void pfctl_init_rule(struct pfctl_rule *r); -int pfctl_append_rule(struct pfctl *, struct pfctl_rule *, const char *); +void pfctl_append_rule(struct pfctl *, struct pfctl_rule *); int pfctl_append_eth_rule(struct pfctl *, struct pfctl_eth_rule *, const char *); int pfctl_add_altq(struct pfctl *, struct pf_altq *);
