Hello,
mikeb pointed out I should not be copy'n'pasting buggy code.
I'm just re-sending updated patch with change below:
--------8<---------------8<---------------8<------------------8<--------
diff -r e2383ec80feb src/sbin/pfctl/pfctl.c
--- a/src/sbin/pfctl/pfctl.c Sat Sep 03 15:39:17 2016 +0200
+++ b/src/sbin/pfctl/pfctl.c Sat Sep 03 16:56:52 2016 +0200
@@ -1076,7 +1076,7 @@
sizeof(rule->anchor->path)) >= sizeof(rule->anchor->path))
errx(1, "pfctl_add_rule: strlcpy");
if ((p = strrchr(anchor_call, '/')) != NULL) {
- if (!strlen(p))
+ if (strlen(p) == 1)
err(1, "pfctl_add_rule: bad anchor name %s",
anchor_call);
} else
@@ -1486,7 +1486,7 @@
errx(1, "pfctl_add_rule: strlcpy");
if ((p = strrchr(anchorname, '/')) != NULL) {
- if (!strlen(p))
+ if (strlen(p) == 1)
err(1, "pfctl_add_rule: bad anchor name %s",
anchorname);
} else
--------8<---------------8<---------------8<------------------8<--------
The problem with strrchr() & strlen() construct is the strrchr() returns
pointer to occurrence of '/' (if it is found). Consider anchorname
contains something like this:
anchorname = "slash/"
the 'p' then gets set to "/", thus !strlen(p) is never satisfied.
The invalid name must travel all the way down to kernel just to
find EINVAL reported by pf_begin_rules()/pf_find_or_create_ruleset().
After this change people should start seeing more helpful message.
updated patch is further below.
thanks and
regards
sasha
--------8<---------------8<---------------8<------------------8<--------
diff -r 8006a1eca673 src/sbin/pfctl/pfctl.c
--- a/src/sbin/pfctl/pfctl.c Sat Sep 03 14:19:17 2016 +0200
+++ b/src/sbin/pfctl/pfctl.c Sat Sep 03 17:12:50 2016 +0200
@@ -1076,7 +1076,7 @@
sizeof(rule->anchor->path)) >= sizeof(rule->anchor->path))
errx(1, "pfctl_add_rule: strlcpy");
if ((p = strrchr(anchor_call, '/')) != NULL) {
- if (!strlen(p))
+ if (strlen(p) == 1)
err(1, "pfctl_add_rule: bad anchor name %s",
anchor_call);
} else
@@ -1342,7 +1342,7 @@
if (path[0])
snprintf(&path[len], PATH_MAX - len, "/%s", pf->anchor->name);
else
- snprintf(&path[len], PATH_MAX - len, "%s", pf->anchor->name);
+ snprintf(&path[len], PATH_MAX - len, "%s", pf->anchor->path);
if (depth) {
if (TAILQ_FIRST(rs->rules.active.ptr) != NULL) {
@@ -1447,6 +1447,7 @@
struct pfr_table trs;
char *path = NULL;
int osize;
+ char *p;
bzero(&pf, sizeof(pf));
RB_INIT(&pf_anchors);
@@ -1483,7 +1484,15 @@
if (strlcpy(pf.anchor->path, anchorname,
sizeof(pf.anchor->path)) >= sizeof(pf.anchor->path))
errx(1, "pfctl_add_rule: strlcpy");
- if (strlcpy(pf.anchor->name, anchorname,
+
+ if ((p = strrchr(anchorname, '/')) != NULL) {
+ if (strlen(p) == 1)
+ err(1, "pfctl_add_rule: bad anchor name %s",
+ anchorname);
+ } else
+ p = anchorname;
+
+ if (strlcpy(pf.anchor->name, p,
sizeof(pf.anchor->name)) >= sizeof(pf.anchor->name))
errx(1, "pfctl_add_rule: strlcpy");
